Install iptables rules to allow wireguard packets #7030

wenyingd · 2025-02-26T02:07:35Z

This change is to actively configure the iptables rules wireguard.port when wireguard is used as the encryption mode. This can fix traffic issues if the Node is configured with iptables default DROP policy.

Fix: #7009

wenyingd · 2025-02-26T02:34:38Z

/test-all
/test-windows-all

wenyingd · 2025-02-26T08:42:39Z

/test-windows-all

XinShuYang · 2025-02-26T11:33:42Z

/test-windows-all

XinShuYang · 2025-02-26T11:33:59Z

/test-windows-all

antoninbas

cc @Dhruv-J who is working on something similar for NodeLatencyMonitor (ICMP filter rules)

pkg/agent/route/route_linux.go

wenyingd · 2025-02-27T09:11:22Z

/test-all
/test-windows-all

pkg/agent/route/route_linux.go

antoninbas · 2025-02-27T21:08:17Z

pkg/agent/route/route_linux_test.go

+			},
+		},
+		{
+			name:                     "encap,wireguard,egress=true,multicastEnabled=true,proxyAll=true,nodeNetworkPolicy=true,nodeSNATRandomFully=true",


would it make the tests a bit less verbose if we removed multicast and proxyAll?

sure, I will update.

I saw the change for the second test case but not this one, was it intentional?

Yes, this case is for the case to test all features enabled. hmm, maybe I should remove the previous one ""encap,egress=true,multicastEnabled=true,proxyAll=true,nodeNetworkPolicy=true,nodeSNATRandomFully=true"?

luolanzone

a few nits

luolanzone · 2025-02-26T02:36:23Z

pkg/agent/route/route_linux.go

+	// wireguardIPTablesIPv4 caches all existing IPv4 iptables chains and rules for Wireguard.
+	wireguardIPTablesIPv4 sync.Map
+	// wireguardIPTablesIPv6 caches all existing IPv6 iptables chains and rules for Wireguard.
+	wireguardIPTablesIPv6 sync.Map


s/Wireguard/WireGuard
please check other occurrences.

Should we add a comment in this doc https://github.com/antrea-io/antrea/blob/main/docs/network-requirements.md to remind users about this change?
@antoninbas any suggestion?

yes, good idea, let's mention it in that doc
we should also mention in the table that most of these ports are configurable (right now we only mention it for the BGP server port)

docs/network-flow-visibility.md

luolanzone · 2025-02-28T03:54:45Z

cmd/antrea-agent/agent.go

@@ -244,7 +244,9 @@ func run(o *Options) error {
 		multicastEnabled,
 		o.config.SNATFullyRandomPorts,
 		*o.config.Egress.SNATFullyRandomPorts,
-		serviceCIDRProvider)
+		serviceCIDRProvider,
+		wireguardConfig.Port,


Another concern is about the WireGuard port check, looks like we didn't check the range before:

if o.config.WireGuard.Port == 0 { o.config.WireGuard.Port = apis.WireGuardListenPort }

I suppose we should go through and update the validation to make sure it's a valid port? @antoninbas

That sounds good to me but we should do it in a separate PR, we have other ports that we could validate as well, e.g. the tunnelPort. Can you open an issue to track this?

Issue created: #7035, I will take a look at it later today.

wenyingd · 2025-02-28T08:12:29Z

/test-all
/test-windows-all

wenyingd · 2025-02-28T11:06:52Z

/test-windows-all

tnqn · 2025-02-28T14:34:54Z

docs/network-requirements.md

-| Antrea with feature BGPPolicy enabled          | Selected by user-provided BGPPolicies | TCP 179<sup>[1]</sup>                      |                              |
-| All                                            | Kube-apiserver host                   | TCP 443 or 6443<sup>[2]</sup>              |                              |
-| All                                            | All                                   | TCP 10349, 10350, 10351, UDP 10351         |                              |
+| Configuration                                  | Host(s)                               | Protocols/Ports                            | Configurable | Other                        |


The table is not displayed properly: https://github.com/antrea-io/antrea/blob/a209673dfb1f8e155f1f644d5295f56351986c48/docs/network-requirements.md

tnqn · 2025-02-28T15:21:08Z

pkg/agent/route/route_linux.go

+	addFilterRulesToChain(iptablesFilterRulesByChainV4, &c.nodeNetworkPolicyIPTablesIPv4)
+	addFilterRulesToChain(iptablesFilterRulesByChainV4, &c.wireguardIPTablesIPv4)


should static rule (wireguard) be installed before the dynamic rules (NodeNetworkPolicy)? this should be slightly more efficient as wireguard traffic will be the majority of node traffic.

updated accordingly.

wenyingd · 2025-03-03T02:56:29Z

/test-all
/test-windows-all

wenyingd · 2025-03-03T06:09:09Z

/test-windows-conformance

XinShuYang · 2025-03-03T06:46:52Z

/test-windows-conformance

antoninbas · 2025-03-03T08:21:49Z

test/integration/agent/route_test.go

@@ -100,7 +100,7 @@ type routeClientOptions struct {
 }

 func newTestRouteClient(networkConfig *config.NetworkConfig, options routeClientOptions) (*route.Client, error) {
-	return route.NewClient(networkConfig, options.noSNAT, false, false, false, false, options.nodeSNATRandomFully, false, nil)
+	return route.NewClient(networkConfig, options.noSNAT, false, false, false, false, options.nodeSNATRandomFully, false, nil, 51820)


maybe use the WireGuardListenPort constant from pkg/apis to make the code more readable

antoninbas · 2025-03-03T08:23:30Z

pkg/agent/route/route_linux_test.go

+			},
+		},
+		{
+			name:                     "encap,wireguard,egress=true,multicastEnabled=true,proxyAll=true,nodeNetworkPolicy=true,nodeSNATRandomFully=true",


I saw the change for the second test case but not this one, was it intentional?

antoninbas · 2025-03-03T08:24:35Z

pkg/agent/route/route_linux.go

+	}
+
+	iptablesFilterRulesByChainV4 := make(map[string][]string)
+	addFilterRulesToChain(iptablesFilterRulesByChainV4, &c.wireguardIPTablesIPv4)


I think we should add a comment explaining that we are intentionally adding static iptables rules (WireGuard for now) first for performance reasons.

antoninbas · 2025-03-03T08:28:09Z

docs/network-requirements.md


 [1] _The default value is 179, but a user created BGPPolicy can assign a different
 port number._

 [2] _The value is passed to kube-apiserver `--secure-port` flag. You can find the port
 number from the output of `kubectl get svc kubernetes -o yaml`._
+
+[3] _Antrea now automatically adds the firewall rules to allow the WireGuard packets,


better to specify the version (starting with v2.4) instead of writing "now" IMO

sure, updated.

wenyingd · 2025-03-03T11:33:09Z

/test-all

tnqn · 2025-03-03T12:51:29Z

docs/network-requirements.md

+| Antrea Multi-cluster with WireGuard encryption | Multi-cluster Gateway Node            | UDP 51821                                  | Yes          |                              |
+| Antrea with feature BGPPolicy enabled          | Selected by user-provided BGPPolicies | TCP 179<sup>[1]</sup>                      | Yes          |                              |
+| All                                            | Kube-apiserver host                   | TCP 443 or 6443<sup>[2]</sup>              | No           |                              |
+| All                                            | All                                   | TCP 10349, 10350, 10351, UDP 10351         | No           |                              |


10349, 10350, and 10351 are all configurable.

Actually even kube-apiserver port is configurable via kube-apiserver itself.

This change is to actively configure the iptables rules wireguard.port when wireguard is used as the encryption mode. This can fix traffic issues if the Node is configured with iptables default DROP policy. Signed-off-by: Wenying Dong <[email protected]>

wenyingd · 2025-03-04T05:26:31Z

/test-all

tnqn

LGTM

antoninbas

LGTM

antoninbas · 2025-03-04T08:01:33Z

docs/network-requirements.md

we should probably try to improve this file in the future, as the configurable column is a bit generic and maybe should indicate how the port can be configured. we could also extract the kube-apiserver requirement from the table and mention it separately, as the requirement doesn't come from Antrea.

wenyingd force-pushed the issue_7009 branch from 253146d to 3ace605 Compare February 26, 2025 02:15

wenyingd requested review from antoninbas, tnqn, hongliangl and luolanzone February 26, 2025 02:31

antoninbas reviewed Feb 26, 2025

View reviewed changes

pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved

pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved

pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved

wenyingd force-pushed the issue_7009 branch from 3ace605 to cb02962 Compare February 27, 2025 02:31

wenyingd changed the title ~~Install iptables rules to allow packets to wireguard port~~ Install iptables rules to allow wireguard packets Feb 27, 2025

wenyingd force-pushed the issue_7009 branch from cb02962 to 428835c Compare February 27, 2025 02:43

hongliangl reviewed Feb 27, 2025

View reviewed changes

wenyingd force-pushed the issue_7009 branch from 428835c to 08553fe Compare February 27, 2025 06:01

hongliangl reviewed Feb 27, 2025

View reviewed changes

pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved

wenyingd force-pushed the issue_7009 branch from 08553fe to b72edf5 Compare February 27, 2025 06:32

wenyingd requested review from hongliangl and antoninbas February 27, 2025 11:13

antoninbas reviewed Feb 27, 2025

View reviewed changes

luolanzone reviewed Feb 28, 2025

View reviewed changes

docs/network-flow-visibility.md Outdated Show resolved Hide resolved

wenyingd force-pushed the issue_7009 branch from b72edf5 to 984f502 Compare February 28, 2025 03:39

luolanzone reviewed Feb 28, 2025

View reviewed changes

wenyingd force-pushed the issue_7009 branch 2 times, most recently from 71afced to a209673 Compare February 28, 2025 06:55

tnqn reviewed Feb 28, 2025

View reviewed changes

wenyingd force-pushed the issue_7009 branch 2 times, most recently from 2e80f16 to fa0e05a Compare March 3, 2025 01:58

wenyingd requested review from tnqn, antoninbas and luolanzone March 3, 2025 02:56

antoninbas reviewed Mar 3, 2025

View reviewed changes

wenyingd force-pushed the issue_7009 branch 2 times, most recently from 36eea72 to 321373e Compare March 3, 2025 10:53

tnqn reviewed Mar 3, 2025

View reviewed changes

wenyingd force-pushed the issue_7009 branch from 321373e to 261134f Compare March 4, 2025 02:46

wenyingd requested a review from tnqn March 4, 2025 05:26

tnqn approved these changes Mar 4, 2025

View reviewed changes

antoninbas approved these changes Mar 4, 2025

View reviewed changes

antoninbas merged commit 525f662 into antrea-io:main Mar 4, 2025
60 checks passed

antoninbas mentioned this pull request Mar 4, 2025

ICMP Rule with NodeLatencyMonitor #7011

Draft

		addFilterRulesToChain(iptablesFilterRulesByChainV4, &c.nodeNetworkPolicyIPTablesIPv4)
		addFilterRulesToChain(iptablesFilterRulesByChainV4, &c.wireguardIPTablesIPv4)

Install iptables rules to allow wireguard packets #7030

Install iptables rules to allow wireguard packets #7030

Conversation

wenyingd commented Feb 26, 2025 • edited Loading

wenyingd commented Feb 26, 2025

wenyingd commented Feb 26, 2025

XinShuYang commented Feb 26, 2025

XinShuYang commented Feb 26, 2025

antoninbas left a comment

Choose a reason for hiding this comment

wenyingd commented Feb 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyingd commented Feb 28, 2025

wenyingd commented Feb 28, 2025

tnqn Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyingd commented Mar 3, 2025

wenyingd commented Mar 3, 2025

XinShuYang commented Mar 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyingd commented Mar 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyingd commented Mar 4, 2025

tnqn left a comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyingd commented Feb 26, 2025 •

edited

Loading

tnqn Feb 28, 2025 •

edited

Loading