Enable L7 DNS proxy for nodes #34024
Conversation
There was a problem hiding this comment.
The inclusion of "host" in your PR as CIDR selectable needs further discussion. I would like to see a lot of this functionality behind the --policy-cidr-match-mode flag as well.
There was a problem hiding this comment.
Nice work! With a couple comments below from my side.
I'd like to see a conclusion to the discussion with @nathanjsweet, I'm not sure why it was marked as “resolved”?
|
Another question: How did you test the feature? It would be nice to get a new test for this in cilium-cli. |
|
Commit b2168e4 does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
I have actually been running the previous 'version' of this feature (#19892) - one that redirected packets with iptables rules rather than eBPF - in our cluster for a long time. The basic smoke test I've been using is to turn on audit mode, create a simple DNS L7 host policy like the one in the PR description, then get the host to make a request allowed by FQDN and one disallowed by FQDN and observe policy verdicts in the output of |
Yes, that would be ideal. |
|
/test |
This commit adds a connectivity test for host L7 DNS proxy capability added in cilium/cilium#34024. Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
|
I added a test to cilium-cli, but I will have to re-fix #23330 in one of these two ways mentioned in that issue
because #24713 changes resolv.conf only in coredns pods, leaving host networking pods with default resolv.conf and making the host-firewall-egress-to-fqdns test I added fail when #23330 is a problem. |
This commit adds a connectivity test for host L7 DNS proxy capability added in cilium/cilium#34024. Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
So if I follow correctly, you add a new test in cilium-cli for the change in the current PR in cilium/cilium-cli#2740, and this test also needs the fix in #34154 to work, is this correct? Very nice work, thanks a lot! 🚀 Looking at the results from CI today (apologies for the delay), I see that we have two failures:
I will be off soon so I'll ask someone else from |
Yes.
I'll try that, thanks. I looked at the logs, but could not make much sense out of it. BPF verifier quirks is not something one can pick up in a day or two. However verifier tests pass for newer kernels (e.g. 6.8.0 on my test VM), maybe the feature can be gated to newer kernels only as a last resort? |
|
/test |
Restored DNS rules code expected that LookupIPsBySecID returns only IP addresses, but if an L7 DNS rule has toCIDR selectors configured, it also returns CIDRs, and after such a DNS rule is restored, it does not match any target DNS servers and the DNS proxy rejects DNS queries until the endpoint is regenerated. This commit fixes this issue, replacing string IP addresses in restore.DNSRules with a wrapped netip.Prefix (preserving JSON serialization of restored rules). Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
The host endpoint currently does not have any IP addresses associated with it. This prevents DNS proxy from working when source is the host endpoint. This commit is a minimal change which enables the host endpoint to be looked up by its IP address without touching any other code. Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
FQDN-based network policy is extremely useful for egress filtering, but is currently only supported in pods. This means that nodes must either rely on some other filtering mechanism, or resort to periodically updating L3/L4 CIDR policies. However, the code which collects DNS requests and combines them with policy to automatically produce L3/L4 filters is at endpoint level and is almost agnostic with respect to kind of endpoint (pod or node). This changeset enables FQDN-based network policy for nodes by adding BPF code which redirects node's DNS requests to the same transparent proxy that is used for pods. Signed-off-by: Anton Tykhyy <atykhyy@gmail.com>
The BPF complexity test was failing on cil-to-netdev function in older kernels under test (5.10 and 6.1) with "BPF program too large" error. I commented out calls to diff --git a/bpf/bpf_host.c b/bpf/bpf_host.c
index e7bf9bd284..89d60ab0ba 100644
--- a/bpf/bpf_host.c
+++ b/bpf/bpf_host.c
@@ -1421,12 +1421,12 @@ int cil_to_netdev(struct __ctx_buff *ctx __maybe_unused)
break;
}
- if (IS_ERR(ret))
- goto drop_err;
-
if (ret == CTX_ACT_REDIRECT)
return ret;
+ if (IS_ERR(ret))
+ goto drop_err;
+
skip_host_firewall:
#endif /* ENABLE_HOST_FIREWALL */ |
|
/test |
|
@atykhyy Do we know if these limitations are still in effect? Cilium operating in CRD mode (as opposed to KVstore mode), Host Policies enabled, tunneling enabled, kube-proxy-replacement enabled, and WireGuard enabled. This combination results in a failure to connect to the clustermesh-apiserver. For details, refer to GitHub issue 31209. Host Policies do not work on host WireGuard interfaces. For details, see GitHub issue 17636. |
FQDN-based network policy is extremely useful for egress filtering, but is currently only supported in pods. This means that nodes must either rely on some other filtering mechanism, or resort to periodically updating L3/L4 CIDR policies. However, the code which collects DNS requests and combines them with policy to automatically produce L3/L4 filters is at endpoint level and is almost agnostic with respect to kind of endpoint (pod or node). This PR enables FQDN-based network policy for nodes by adding a bit of BPF code which, when instructed by network policy, redirects node's DNS requests to the same transparent proxy that is used for pods. No changes in iptables rules or routing are required.
FQDN-based network policy for nodes is likely to be a good addition to Cilium, as it would enable egress filtering by FQDNs for the whole cluster without relying on third-party solutions. Also, the same approach may possibly work for other L7 rules.
A sample policy I used for testing is