Skip to content

Verify source addresses of egress NAT traffic - #388

Open
chez-shanpu wants to merge 2 commits into
mainfrom
egress-src-verify
Open

chez-shanpu wants to merge 2 commits into
mainfrom
egress-src-verify

Conversation

@chez-shanpu

@chez-shanpu chez-shanpu commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR hardens egress NAT by verifying the source addresses of tunneled packets on both NAT servers and NAT clients.
NAT servers now accept packets only from registered client Pods, and only when the source address is the client Pod's own IP address.
NAT clients accept only returning packets of the connections they initiated.
The number of packets dropped by the source address verification on NAT servers is exported as metrics.

Changes

NAT server (coil-egress)

  • NatServer.AddClient installs a rule in both the FORWARD and INPUT chains. The rule drops packets from the client's FoU link unless their source address is the client Pod's IP address.
    • iptables: -i fou4_<hex> ! -s <clientIP> -m comment --comment coil-peer-src-fou4_<hex> -j DROP
    • nftables: iifname "fou4_<hex>" ip saddr != <clientIP> counter drop (tagged with coil-peer-src-fou4_<hex> as UserData)
  • The rule is installed before the link is brought up.
  • New method NatServer.DelClient removes the rule. podWatcher calls it right after deleting the FoU link.
  • NAT servers no longer create the flow based IPIP devices (coil_ipip4 / coil_ipip6), so they decapsulate packets only from registered peers.

NAT client (coild)

  • NewFoUTunnel is split into NewClientFoUTunnel and NewServerFoUTunnel.
    • Only the server role enables IP forwarding.
    • Only the client role sets up the flow based IPIP devices to receive returning packets from egress Pods.
  • NatClient drops new flows (ct state new) from the flow based devices in the FORWARD and INPUT chains.
  • EgressWatcher calls the new NatClient.EnsureFlowBasedDevRules so that client Pods that already exist get these rules when coild is upgraded.

Metrics (coil-egress)

  • New metrics coil_egress_nftables_peer_src_dropped_packets_total and coil_egress_nftables_peer_src_dropped_bytes_total. They are the sums of the counters of the source address verification rules in the FORWARD and INPUT chains, with the same labels as the existing metrics.
  • The rule for invalid packets is now tagged with coil-invalid-drop (UserData for nftables, a comment for iptables). coil_egress_nftables_invalid_{packets,bytes}_total find the rule by the tag instead of taking the first rule with a counter in the FORWARD chain, so they no longer depend on the order of the rules.
  • The tags are exported from the netfilter package as RuleTagInvalidDrop and RuleTagPeerSrcPrefix.

Docs

  • Added "Source address verification" to docs/usage.md.
  • Added the new metrics to docs/cmd-coil-egress.md.

Behavior changes

  • Packets from a client Pod whose source address is not the Pod IP are dropped. If a client Pod forwards traffic from nested network namespaces or containers through Egress, that traffic must be SNATed to the Pod IP. Docker's default masquerade rule does this.
  • Traffic from client Pods to the Egress Service's ClusterIP must not be SNATed. The service proxy must not masquerade traffic from Pods to ClusterIPs, for example with kube-proxy --masquerade-all, or for client Pod addresses outside --cluster-cidr.
  • NAT clients no longer enable IP forwarding by themselves.

Tests

  • pkg/nat/netfilter: rule setup, idempotency and removal in both chains, and the tags of the rules. EnsureFlowBasedDevRules is also tested without a prior Init.
  • pkg/fou: tests for the client and server roles.
  • pkg/metrics: unit tests for finding rules by tags (both the nftables UserData and the iptables comment) and summing their counters.
  • pkg/test: integration tests checking that packets with an unexpected source address from a registered client, and FoU packets from an unregistered peer, are dropped.
  • controllers: the nat.Server mock gains DelClient, and the pod watcher tests now check that clients are unregistered.

🤖 Generated with Claude Code

@chez-shanpu
chez-shanpu requested a review from terassyi October 21, 2026 05:53
…ting packets from unknown peers

Signed-off-by: Tomoki Sugiura <tomoki-sugiura@cybozu.co.jp>
@terassyi

terassyi commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

@chez-shanpu
I've reviewed the changes, it looks mostly good to me.

I have a suggestion about metrics.
This PR adds counter drop rules to FORWARD and INPUT, but pkg/metrics/egress.go is not changed. Two points I would like to see addressed in this PR:

  1. Spoofed-source drops are not exported. The nftables peer-src rules have a counter and a coil-peer-src-<link> UserData tag, and GetRules returns both, but nothing reads them. Please sum them and export a new metric such as coil_egress_nftables_peer_src_dropped_packets_total with the same labels as the existing ones. For the iptables backend, adding -m comment to the rules makes them easy to find: the comment shows up as an xt comment match in the nft API, and in iptables -L -v -x.

  2. The invalid counter now depends on rule order. getNfTablesInvalidCounter returns the first rule in FORWARD that has a counter. The comment says there is only one such rule (pkg/metrics/egress.go:197), but after this PR there are more. It still works because NewNatServer installs the invalid rule before any AddClient. With iptables-nft every rule has a counter, so this is really "the first rule in the chain". Please add a UserData tag to the invalid rule and look it up by the tag.

Signed-off-by: Tomoki Sugiura <tomoki-sugiura@cybozu.co.jp>
@chez-shanpu

Copy link
Copy Markdown
Contributor Author

@terassyi Thank you! I've addressed both of them. Can you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants