Skip to content

Commit

Permalink
Always enable global IPv6 forwarding
Browse files Browse the repository at this point in the history 
Global forwarding works differently for IPv6:
  conf/all/forwarding - BOOLEAN
   Enable global IPv6 forwarding between all interfaces.
	  IPv4 and IPv6 work differently here; e.g. netfilter must be used
	  to control which interfaces may forward packets and which not.
https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt

It is not possible to configure the IPv6 forwarding per interface by
setting the net.ipv6.conf.<ifname>.forwarding sysctl. Instead,
the opposite approach is required where the global forwarding
is enabled and an iptables policy is added to restrict it by default.

To ensure consistent behavior between IPv4/IPv6 and limit the
forwarding scope for IPv4 networks this commit configures the default
DROP policy for all configured IP families.

Signed-off-by: Patryk Diak <pdiak@redhat.com>
  • Loading branch information
@kyrtapz
kyrtapz committed Jun 17, 2024
1 parent 701b8e0 commit 575c6dd
Show file tree
Hide file tree
Showing 12 changed files with 240 additions and 204 deletions.
107 changes: 91 additions & 16 deletions docs/getting-started/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,21 @@ Let us look at the supported configuration variables by OVN-Kubernetes

### Disable Forwarding Config

OVN-Kubernetes allows to enable or disable IP forwarding for all traffic on OVN-Kubernetes managed interfaces (such as br-ex). By default forwarding is enabled and this allows host to forward traffic across OVN-Kubernetes managed interfaces. If forwarding is disabled then Kubernetes related traffic is still forwarded appropriately, but other IP traffic will not be routed by cluster nodes.
OVN-Kubernetes allows to enable or disable IP forwarding for all traffic on OVN-Kubernetes managed interfaces (such as br-ex).
By default forwarding is enabled and this allows host to forward traffic across OVN-Kubernetes managed interfaces.
If forwarding is disabled then Kubernetes related traffic is still forwarded appropriately, but other IP traffic will not be routed by cluster nodes.

IP forwarding is implemented at cluster node level by modifying both iptables `FORWARD` chain and IP forwarding `sysctl` parameters.

- If forwarding is enabled(default) then system administrators need to set following sysctl parameters. An operator can be built to manage forwarding sysctl parameters based on forwarding mode. No extra iptables rules are added by OVN-Kubernetes to FORWARD chain while using this IP forwarding mode.
#### IPv4

If forwarding is enabled(default) then system administrators need to set following sysctl parameters. An operator can be built to manage forwarding sysctl parameters based on forwarding mode. No extra iptables rules are added by OVN-Kubernetes to FORWARD chain while using this IP forwarding mode.
```
net.ipv4.ip_forward=1
net.ipv6.conf.all.forwarding=1
```

- IP forwarding can be disabled either by setting `disable-forwarding` command line option to `true` while starting ovnkube or by setting `disable-forwarding` to `true` in config file. If forwarding is disabled then system administrators need to set following sysctl parameters to stop routing other IP traffic. An operator can be built to manage forwarding sysctl parameters based on forwarding mode.

IP forwarding can be disabled either by setting `disable-forwarding` command line option to `true` while starting ovnkube or by setting `disable-forwarding` to `true` in config file. If forwarding is disabled then system administrators need to set following sysctl parameters to stop routing other IP traffic. An operator can be built to manage forwarding sysctl parameters based on forwarding mode.
```
net.ipv4.ip_forward=0
net.ipv6.conf.all.forwarding=0
```

When IP forwarding is disabled, following sysctl parameters are modified by OVN-Kubernetes to allow forwarding Kubernetes related traffic on OVN-Kubernetes managed bridge interfaces and management port interface.
Expand All @@ -33,17 +32,93 @@ net.ipv4.conf.br-ex.forwarding=1
net.ipv4.conf.ovn-k8s-mp0.forwarding = 1
```

Additionally following iptables rules are added at FORWARD chain to forward clusterNetwork and serviceNetwork traffic to their intended destinations.
#### IPv6
IP forwarding works differently for IPv6:
```
/proc/sys/net/ipv6/* Variables:
conf/all/forwarding - BOOLEAN
Enable global IPv6 forwarding between all interfaces.
IPv4 and IPv6 work differently here; e.g. netfilter must be used
to control which interfaces may forward packets and which not.
...
conf/interface/*:
forwarding - INTEGER
Configure interface-specific Host/Router behaviour.
Note: It is recommended to have the same setting on all
interfaces; mixed router/host scenarios are rather uncommon.
Possible values are:
0 Forwarding disabled
1 Forwarding enabled
FALSE (0):
By default, Host behaviour is assumed. This means:
1. IsRouter flag is not set in Neighbour Advertisements.
2. If accept_ra is TRUE (default), transmit Router
Solicitations.
3. If accept_ra is TRUE (default), accept Router
Advertisements (and do autoconfiguration).
4. If accept_redirects is TRUE (default), accept Redirects.
TRUE (1):
If local forwarding is enabled, Router behaviour is assumed.
This means exactly the reverse from the above:
1. IsRouter flag is set in Neighbour Advertisements.
2. Router Solicitations are not sent unless accept_ra is 2.
3. Router Advertisements are ignored unless accept_ra is 2.
4. Redirects are ignored.
Default: 0 (disabled) if global forwarding is disabled (default),
otherwise 1 (enabled).
```
https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt

It is not possible to configure the IPv6 forwarding per interface by setting the
`net.ipv6.conf.IFNAME.forwarding` sysctl (it just configures the interface-specific Host/Router
behaviour). \
Instead, the opposite approach is required where the global forwarding is always enabled and
the traffic is restricted through iptables.

#### forwarding rules
When the `disable-forwarding` parameter is configured specific iptables rules are added to
the FORWARD chain to forward clusterNetwork and serviceNetwork traffic to their intended destinations. \
Additionally, the default policy for the FORWARD chain is set as `DROP`. Otherwise, the policy
defaults to `ALLOW` and no custom rules are added. This behavior is the same for both IPv6 and IPv4
networks:

```
-A FORWARD -s 10.128.0.0/14 -j ACCEPT
-A FORWARD -d 10.128.0.0/14 -j ACCEPT
-A FORWARD -s 169.254.169.1 -j ACCEPT
-A FORWARD -d 169.254.169.1 -j ACCEPT
-A FORWARD -d 172.16.1.0/24 -j ACCEPT
-A FORWARD -s 172.16.1.0/24 -j ACCEPT
-A FORWARD -i breth1 -j DROP
-A FORWARD -o breth1 -j DROP
# In IPv4 with disable-forwarding=true the FORWARD policy is set to DROP
Chain FORWARD (policy DROP 0 packets, 0 bytes)
pkts bytes target prot opt in out source destination
0 0 ACCEPT 0 -- * * 0.0.0.0/0 169.254.169.1
0 0 ACCEPT 0 -- * * 169.254.169.1 0.0.0.0/0
0 0 ACCEPT 0 -- * * 0.0.0.0/0 10.96.0.0/16
0 0 ACCEPT 0 -- * * 10.96.0.0/16 0.0.0.0/0
0 0 ACCEPT 0 -- * * 0.0.0.0/0 10.244.0.0/16
0 0 ACCEPT 0 -- * * 10.244.0.0/16 0.0.0.0/0
0 0 ACCEPT 0 -- ovn-k8s-mp0 * 0.0.0.0/0 0.0.0.0/0
0 0 ACCEPT 0 -- * ovn-k8s-mp0 0.0.0.0/0 0.0.0.0/0
# In IPv6 with disable-forwarding=true the FORWARD policy is set to DROP
Chain FORWARD (policy DROP 0 packets, 0 bytes)
pkts bytes target prot opt in out source destination
0 0 ACCEPT 0 -- * * ::/0 fd69::1
0 0 ACCEPT 0 -- * * fd69::1 ::/0
0 0 ACCEPT 0 -- * * ::/0 fd00:10:96::/112
0 0 ACCEPT 0 -- * * fd00:10:96::/112 ::/0
0 0 ACCEPT 0 -- * * ::/0 fd00:10:244::/48
0 0 ACCEPT 0 -- * * fd00:10:244::/48 ::/0
0 0 ACCEPT 0 -- ovn-k8s-mp0 * ::/0 ::/0
0 0 ACCEPT 0 -- * ovn-k8s-mp0 ::/0 ::/0
```

## Logging Config
Expand Down
47 changes: 46 additions & 1 deletion go-controller/pkg/node/default_node_network_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"

"github.com/containernetworking/plugins/pkg/ip"
v1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
honode "github.com/ovn-org/ovn-kubernetes/go-controller/hybrid-overlay/pkg/controller"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni"
Expand All @@ -40,7 +41,6 @@ import (
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
utilerrors "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util/errors"

"github.com/containernetworking/plugins/pkg/ip"
"github.com/vishvananda/netlink"
)

Expand Down Expand Up @@ -704,6 +704,10 @@ func (nc *DefaultNodeNetworkController) Start(ctx context.Context) error {
nc.routeManager.Run(nc.stopChan, 4*time.Minute)
}()

if err = configureGlobalForwarding(); err != nil {
return err
}

// Bootstrap flows in OVS if just normal flow is present
if err := bootstrapOVSFlows(); err != nil {
return fmt.Errorf("failed to bootstrap OVS flows: %w", err)
Expand Down Expand Up @@ -1330,3 +1334,44 @@ func DummyNextHopIPs() []net.IP {
}
return nextHops
}

// configureGlobalForwarding configures the global forwarding settings.
// It sets the FORWARD policy to DROP/ALLOW based on the config.Gateway.DisableForwarding value for all enabled IP families.
// For IPv6 it additionally always enables the global forwarding.
func configureGlobalForwarding() error {
// Global forwarding works differently for IPv6:
// conf/all/forwarding - BOOLEAN
// Enable global IPv6 forwarding between all interfaces.
// IPv4 and IPv6 work differently here; e.g. netfilter must be used
// to control which interfaces may forward packets and which not.
// https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
//
// It is not possible to configure the IPv6 forwarding per interface by
// setting the net.ipv6.conf.<ifname>.forwarding sysctl. Instead,
// the opposite approach is required where the global forwarding
// is enabled and an iptables rule is added to restrict it by default.
if config.IPv6Mode {
if err := ip.EnableIP6Forward(); err != nil {
return fmt.Errorf("could not set the correct global forwarding value for ipv6: %w", err)
}

}

for _, proto := range clusterIPTablesProtocols() {
ipt, err := util.GetIPTablesHelper(proto)
if err != nil {
return fmt.Errorf("failed to get the iptables helper: %w", err)
}

if config.Gateway.DisableForwarding {
if err := ipt.ChangePolicy("filter", "FORWARD", "DROP"); err != nil {
return fmt.Errorf("failed to change the forward policy: %w", err)
}
} else {
if err := ipt.ChangePolicy("filter", "FORWARD", "ACCEPT"); err != nil {
return fmt.Errorf("failed to change the forward policy: %w", err)
}
}
}
return nil
}
14 changes: 7 additions & 7 deletions go-controller/pkg/node/egress_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ var _ = Describe("Egress Service Operations", func() {

f4 := iptV4.(*util.FakeIPTables)
Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

Expect(fakeOvnNode.fakeExec.CalledMatchesExpected()).To(BeTrue(), fakeOvnNode.fakeExec.ErrorDesc)
Expand Down Expand Up @@ -393,7 +393,7 @@ var _ = Describe("Egress Service Operations", func() {

f4 := iptV4.(*util.FakeIPTables)
Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

expectedTables = map[string]util.FakeTable{
Expand All @@ -408,7 +408,7 @@ var _ = Describe("Egress Service Operations", func() {
Expect(err).ToNot(HaveOccurred())

Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

Expect(fakeOvnNode.fakeExec.CalledMatchesExpected()).To(BeTrue(), fakeOvnNode.fakeExec.ErrorDesc)
Expand Down Expand Up @@ -536,7 +536,7 @@ var _ = Describe("Egress Service Operations", func() {
}
f4 := iptV4.(*util.FakeIPTables)
Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

expectedTables = map[string]util.FakeTable{
Expand All @@ -551,7 +551,7 @@ var _ = Describe("Egress Service Operations", func() {
Expect(err).ToNot(HaveOccurred())

Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

Expect(fakeOvnNode.fakeExec.CalledMatchesExpected()).To(BeTrue(), fakeOvnNode.fakeExec.ErrorDesc)
Expand Down Expand Up @@ -677,7 +677,7 @@ var _ = Describe("Egress Service Operations", func() {
}
f4 := iptV4.(*util.FakeIPTables)
Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

Eventually(func() bool {
Expand All @@ -697,7 +697,7 @@ var _ = Describe("Egress Service Operations", func() {
Expect(err).ToNot(HaveOccurred())

Eventually(func() error {
return f4.MatchState(expectedTables)
return f4.MatchState(expectedTables, nil)
}).ShouldNot(HaveOccurred())

Eventually(func() bool {
Expand Down
21 changes: 5 additions & 16 deletions go-controller/pkg/node/gateway_init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,12 @@ import (
// bridgedGatewayNodeSetup enables forwarding on bridge interface, sets up the physical network name mappings for the bridge,
// and returns an ifaceID created from the bridge name and the node name
func bridgedGatewayNodeSetup(nodeName, bridgeName, physicalNetworkName string) (string, error) {
// enable forwarding on bridge interface always
createForwardingRule := func(family string) error {
stdout, stderr, err := util.RunSysctl("-w", fmt.Sprintf("net.%s.conf.%s.forwarding=1", family, bridgeName))
if err != nil || stdout != fmt.Sprintf("net.%s.conf.%s.forwarding = 1", family, bridgeName) {
return fmt.Errorf("could not set the correct forwarding value for interface %s: stdout: %v, stderr: %v, err: %v",
bridgeName, stdout, stderr, err)
}
return nil
}
// IPv6 forwarding is enabled globally
if config.IPv4Mode {
if err := createForwardingRule("ipv4"); err != nil {
return "", fmt.Errorf("could not add IPv4 forwarding rule: %v", err)
}
}
if config.IPv6Mode {
if err := createForwardingRule("ipv6"); err != nil {
return "", fmt.Errorf("could not add IPv6 forwarding rule: %v", err)
stdout, stderr, err := util.RunSysctl("-w", fmt.Sprintf("net.ipv4.conf.%s.forwarding=1", bridgeName))
if err != nil || stdout != fmt.Sprintf("net.ipv4.conf.%s.forwarding = 1", bridgeName) {
return "", fmt.Errorf("could not set the correct forwarding value for interface %s: stdout: %v, stderr: %v, err: %v",
bridgeName, stdout, stderr, err)
}
}

Expand Down
29 changes: 11 additions & 18 deletions go-controller/pkg/node/gateway_init_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,7 @@ func shareGatewayInterfaceTest(app *cli.App, testNS ns.NetNS,
Output: "net.ipv4.conf.breth0.forwarding = 1",
})
}
if config.IPv6Mode {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "sysctl -w net.ipv6.conf.breth0.forwarding=1",
Output: "net.ipv6.conf.breth0.forwarding = 1",
})
}

fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=15 --if-exists get interface breth0 mac_in_use",
Output: eth0MAC,
Expand Down Expand Up @@ -378,7 +373,7 @@ func shareGatewayInterfaceTest(app *cli.App, testNS ns.NetNS,
},
}
f4 := iptV4.(*util.FakeIPTables)
err = f4.MatchState(expectedTables)
err = f4.MatchState(expectedTables, nil)
Expect(err).NotTo(HaveOccurred())

expectedTables = map[string]util.FakeTable{
Expand All @@ -387,7 +382,7 @@ func shareGatewayInterfaceTest(app *cli.App, testNS ns.NetNS,
"mangle": {},
}
f6 := iptV6.(*util.FakeIPTables)
err = f6.MatchState(expectedTables)
err = f6.MatchState(expectedTables, nil)
Expect(err).NotTo(HaveOccurred())
return nil
}
Expand Down Expand Up @@ -909,12 +904,7 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
Output: "net.ipv4.conf.breth0.forwarding = 1",
})
}
if config.IPv6Mode {
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "sysctl -w net.ipv6.conf.breth0.forwarding=1",
Output: "net.ipv6.conf.breth0.forwarding = 1",
})
}

fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-vsctl --timeout=15 --if-exists get interface breth0 mac_in_use",
Output: eth0MAC,
Expand Down Expand Up @@ -1084,6 +1074,8 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
})
err = testNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()

Expect(configureGlobalForwarding()).To(Succeed())
gatewayNextHops, gatewayIntf, err := getGatewayNextHops()
Expect(err).NotTo(HaveOccurred())
ifAddrs := ovntest.MustParseIPNets(eth0CIDR)
Expand Down Expand Up @@ -1183,8 +1175,6 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
"-s 10.1.0.0/16 -j ACCEPT",
"-i ovn-k8s-mp0 -j ACCEPT",
"-o ovn-k8s-mp0 -j ACCEPT",
"-i breth0 -j DROP",
"-o breth0 -j DROP",
},
"INPUT": []string{
"-i ovn-k8s-mp0 -m comment --comment from OVN to localhost -j ACCEPT",
Expand All @@ -1198,7 +1188,10 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
},
}
f4 := iptV4.(*util.FakeIPTables)
err = f4.MatchState(expectedTables)
err = f4.MatchState(expectedTables, map[util.FakePolicyKey]string{util.FakePolicyKey{
Table: "filter",
Chain: "FORWARD",
}: "DROP"})
Expect(err).NotTo(HaveOccurred())

expectedTables = map[string]util.FakeTable{
Expand All @@ -1207,7 +1200,7 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
"mangle": {},
}
f6 := iptV6.(*util.FakeIPTables)
err = f6.MatchState(expectedTables)
err = f6.MatchState(expectedTables, nil)
Expect(err).NotTo(HaveOccurred())
return nil
}
Expand Down
Loading

0 comments on commit 575c6dd

Please sign in to comment.