Skip to content

Commit ea10061

Browse files
kyrtapzkyrtapz
committed
Block local processes from accessing the UDNs subnets
An important caveat is that this rule will also block the VRF from accessing the UDN subnets. Additionally modify the isolation E2Es to use Consitently instead of Eventually for cases where no connectivity is expected. Signed-off-by: Patryk Diak <[email protected]>
1 parent 981742d commit ea10061

File tree

4 files changed

+340
-46
lines changed

4 files changed

+340
-46
lines changed

‎go-controller/pkg/node/gateway_shared_intf.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ const (
6969
// nftablesUDNMarkExternalIPsV4Map, nftablesUDNMarkExternalIPsV6Map
7070
nftablesUDNServiceMarkChain = "udn-service-mark"
7171

72+
// nftablesUDNBGPOutputChain is a base chain used for blocking the local processes
73+
// from accessing any of the advertised UDN networks
74+
nftablesUDNBGPOutputChain = "udn-bgp-drop"
75+
76+
// nftablesAdvertisedUDNsSetV[4|6] is a set containing advertised UDN subnets
77+
nftablesAdvertisedUDNsSetV4 = "advertised-udn-subnets-v4"
78+
nftablesAdvertisedUDNsSetV6 = "advertised-udn-subnets-v6"
79+
7280
// nftablesUDNMarkNodePortsMap is a verdict maps containing
7381
// localNodeIP / protocol / port keys indicating traffic that
7482
// should be marked with a UDN specific value, which is used to direct the traffic
@@ -2457,6 +2465,11 @@ func newNodePortWatcher(
24572465
return nil, fmt.Errorf("unable to configure UDN nftables: %w", err)
24582466
}
24592467
}
2468+
if util.IsRouteAdvertisementsEnabled() {
2469+
if err := configureAdvertisedUDNIsolationNFTables(); err != nil {
2470+
return nil, fmt.Errorf("unable to configure UDN isolation nftables: %w", err)
2471+
}
2472+
}
24602473
}
24612474

24622475
var subnets []*net.IPNet
@@ -2919,3 +2932,74 @@ func getIPv(ipnet *net.IPNet) string {
29192932
}
29202933
return prefix
29212934
}
2935+
2936+
// configureAdvertisedUDNIsolationNFTables configures nftables to drop traffic generated locally towards advertised UDN subnets.
2937+
// It sets up a nftables chain named nftablesUDNBGPOutputChain in the output hook with filter priority which drops
2938+
// traffic originating from the local node destined to nftablesAdvertisedUDNsSet.
2939+
// It creates nftablesAdvertisedUDNsSet[v4|v6] set which stores the subnets.
2940+
// Results in:
2941+
//
2942+
// set advertised-udn-subnets-v4 {
2943+
// type ipv4_addr
2944+
// flags interval
2945+
// comment "advertised UDN V4 subnets"
2946+
// }
2947+
// set advertised-udn-subnets-v6 {
2948+
// type ipv6_addr
2949+
// flags interval
2950+
// comment "advertised UDN V6 subnets"
2951+
// }
2952+
// chain udn-bgp-drop {
2953+
// comment "Drop traffic generated locally towards advertised UDN subnets"
2954+
// type filter hook output priority filter; policy accept;
2955+
// ip daddr @advertised-udn-subnets-v4 counter packets 0 bytes 0 drop
2956+
// ip6 daddr @advertised-udn-subnets-v6 counter packets 0 bytes 0 drop
2957+
// }
2958+
func configureAdvertisedUDNIsolationNFTables() error {
2959+
counterIfDebug := ""
2960+
if config.Logging.Level > 4 {
2961+
counterIfDebug = "counter"
2962+
}
2963+
2964+
nft, err := nodenft.GetNFTablesHelper()
2965+
if err != nil {
2966+
return err
2967+
}
2968+
tx := nft.NewTransaction()
2969+
tx.Add(&knftables.Chain{
2970+
Name: nftablesUDNBGPOutputChain,
2971+
Comment: knftables.PtrTo("Drop traffic generated locally towards advertised UDN subnets"),
2972+
2973+
Type: knftables.PtrTo(knftables.FilterType),
2974+
Hook: knftables.PtrTo(knftables.OutputHook),
2975+
Priority: knftables.PtrTo(knftables.FilterPriority),
2976+
})
2977+
tx.Flush(&knftables.Chain{Name: nftablesUDNBGPOutputChain})
2978+
2979+
// TODO: clean up any stale entries in advertised-udn-subnets-v[4|6]
2980+
set := &knftables.Set{
2981+
Name: nftablesAdvertisedUDNsSetV4,
2982+
Comment: knftables.PtrTo("advertised UDN V4 subnets"),
2983+
Type: "ipv4_addr",
2984+
Flags: []knftables.SetFlag{knftables.IntervalFlag},
2985+
}
2986+
tx.Add(set)
2987+
2988+
set = &knftables.Set{
2989+
Name: nftablesAdvertisedUDNsSetV6,
2990+
Comment: knftables.PtrTo("advertised UDN V6 subnets"),
2991+
Type: "ipv6_addr",
2992+
Flags: []knftables.SetFlag{knftables.IntervalFlag},
2993+
}
2994+
tx.Add(set)
2995+
2996+
tx.Add(&knftables.Rule{
2997+
Chain: nftablesUDNBGPOutputChain,
2998+
Rule: knftables.Concat(fmt.Sprintf("ip daddr @%s", nftablesAdvertisedUDNsSetV4), counterIfDebug, "drop"),
2999+
})
3000+
tx.Add(&knftables.Rule{
3001+
Chain: nftablesUDNBGPOutputChain,
3002+
Rule: knftables.Concat(fmt.Sprintf("ip6 daddr @%s", nftablesAdvertisedUDNsSetV6), counterIfDebug, "drop"),
3003+
})
3004+
return nft.Run(context.TODO(), tx)
3005+
}

‎go-controller/pkg/node/gateway_udn.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,14 @@ func (udng *UserDefinedNetworkGateway) DelNetwork() error {
437437
return fmt.Errorf("failed to reconcile default gateway for network %s, err: %v", udng.GetNetworkName(), err)
438438
}
439439
}
440+
441+
if util.IsPodNetworkAdvertisedAtNode(udng.NetInfo, udng.node.Name) {
442+
err := udng.updateAdvertisedUDNIsolationRules(false)
443+
if err != nil {
444+
return fmt.Errorf("failed to remove advertised UDN isolation rules for network %s: %w", udng.GetNetworkName(), err)
445+
}
446+
}
447+
440448
if err := udng.delMarkChain(); err != nil {
441449
return err
442450
}
@@ -918,6 +926,9 @@ func (udng *UserDefinedNetworkGateway) doReconcile() error {
918926
return fmt.Errorf("error while updating logical flow for UDN %s: %s", udng.GetNetworkName(), err)
919927
}
920928

929+
if err := udng.updateAdvertisedUDNIsolationRules(isNetworkAdvertised); err != nil {
930+
return fmt.Errorf("error while updating advertised UDN isolation rules for network %s: %w", udng.GetNetworkName(), err)
931+
}
921932
return nil
922933
}
923934

@@ -975,3 +986,74 @@ func (udng *UserDefinedNetworkGateway) removeDefaultRouteFromVRF() error {
975986
}
976987
return nil
977988
}
989+
990+
// updateAdvertisedUDNIsolationRules adds the full UDN subnets to nftablesAdvertisedUDNsSetV[4|6] nft set that is used
991+
// in the following chain/rules to drop locally generated traffic towards a UDN network:
992+
//
993+
// chain udn-bgp-drop {
994+
// comment "Drop traffic generated locally towards advertised UDN subnets"
995+
// type filter hook output priority filter; policy accept;
996+
// ip daddr @advertised-udn-subnets-v4 counter packets 0 bytes 0 drop
997+
// ip6 daddr @advertised-udn-subnets-v6 counter packets 0 bytes 0 drop
998+
// }
999+
//
1000+
// It blocks access to the full UDN subnet to handle a case in L3 when a node tries to access
1001+
// a host subnet available on a different node. Example set entries:
1002+
//
1003+
// set advertised-udn-subnets-v4 {
1004+
// type ipv4_addr
1005+
// flags interval
1006+
// comment "advertised UDNs V4 subnets"
1007+
// elements = { 10.10.0.0/16 comment "cluster_udn_l3network" }
1008+
// }
1009+
func (udng *UserDefinedNetworkGateway) updateAdvertisedUDNIsolationRules(isNetworkAdvertised bool) error {
1010+
nft, err := nodenft.GetNFTablesHelper()
1011+
if err != nil {
1012+
return fmt.Errorf("failed to get nftables helper: %v", err)
1013+
}
1014+
tx := nft.NewTransaction()
1015+
1016+
if !isNetworkAdvertised {
1017+
existingV4, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV4)
1018+
if err != nil {
1019+
if !knftables.IsNotFound(err) {
1020+
return fmt.Errorf("could not list existing items in %s set: %w", nftablesAdvertisedUDNsSetV4, err)
1021+
}
1022+
}
1023+
existingV6, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV6)
1024+
if err != nil {
1025+
if !knftables.IsNotFound(err) {
1026+
return fmt.Errorf("could not list existing items in %s set: %w", nftablesAdvertisedUDNsSetV6, err)
1027+
}
1028+
}
1029+
1030+
for _, elem := range append(existingV4, existingV6...) {
1031+
if elem.Comment != nil && *elem.Comment == udng.GetNetworkName() {
1032+
tx.Delete(elem)
1033+
}
1034+
}
1035+
1036+
if tx.NumOperations() == 0 {
1037+
return nil
1038+
}
1039+
return nft.Run(context.TODO(), tx)
1040+
}
1041+
1042+
for _, udnNet := range udng.Subnets() {
1043+
set := nftablesAdvertisedUDNsSetV4
1044+
if utilnet.IsIPv6CIDR(udnNet.CIDR) {
1045+
set = nftablesAdvertisedUDNsSetV6
1046+
}
1047+
tx.Add(&knftables.Element{
1048+
Set: set,
1049+
Key: []string{udnNet.CIDR.String()},
1050+
Comment: knftables.PtrTo(udng.GetNetworkName()),
1051+
})
1052+
1053+
}
1054+
1055+
if tx.NumOperations() == 0 {
1056+
return nil
1057+
}
1058+
return nft.Run(context.TODO(), tx)
1059+
}

‎go-controller/pkg/node/gateway_udn_test.go‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package node
22

33
import (
4+
"context"
45
"fmt"
56
"net"
67
"sort"
@@ -11,6 +12,7 @@ import (
1112

1213
"github.com/containernetworking/plugins/pkg/ns"
1314
"github.com/containernetworking/plugins/pkg/testutils"
15+
nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
1416
nadfake "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned/fake"
1517
"github.com/stretchr/testify/mock"
1618
"github.com/vishvananda/netlink"
@@ -21,6 +23,7 @@ import (
2123
"k8s.io/client-go/kubernetes/fake"
2224
utilnet "k8s.io/utils/net"
2325
"k8s.io/utils/ptr"
26+
"sigs.k8s.io/knftables"
2427

2528
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
2629
udnfakeclient "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/userdefinednetwork/v1/apis/clientset/versioned/fake"
@@ -1652,3 +1655,137 @@ func TestConstructUDNVRFIPRulesPodNetworkAdvertised(t *testing.T) {
16521655
})
16531656
}
16541657
}
1658+
1659+
func TestUserDefinedNetworkGateway_updateAdvertisedUDNIsolationRules(t *testing.T) {
1660+
tests := []struct {
1661+
name string
1662+
nad *nadapi.NetworkAttachmentDefinition
1663+
isNetworkAdvertised bool
1664+
initialElements []*knftables.Element
1665+
expectedV4Elements []*knftables.Element
1666+
expectedV6Elements []*knftables.Element
1667+
}{
1668+
{
1669+
name: "Should add V4 and V6 entries to the set for advertised L3 network",
1670+
nad: ovntest.GenerateNAD("test", "rednad", "greenamespace",
1671+
types.Layer3Topology, "100.128.0.0/16/24,ae70::/60/64", types.NetworkRolePrimary),
1672+
isNetworkAdvertised: true,
1673+
expectedV4Elements: []*knftables.Element{{
1674+
Set: nftablesAdvertisedUDNsSetV4,
1675+
Key: []string{"100.128.0.0/16"},
1676+
Comment: knftables.PtrTo[string]("test"),
1677+
}},
1678+
expectedV6Elements: []*knftables.Element{{
1679+
Set: nftablesAdvertisedUDNsSetV6,
1680+
Key: []string{"ae70::/60"},
1681+
Comment: knftables.PtrTo[string]("test"),
1682+
}},
1683+
},
1684+
{
1685+
name: "Should add V4 and V6 entries to the set for advertised L2 network",
1686+
nad: ovntest.GenerateNAD("test", "rednad", "greenamespace",
1687+
types.Layer2Topology, "100.128.0.0/16,ae70::/60", types.NetworkRolePrimary),
1688+
isNetworkAdvertised: true,
1689+
expectedV4Elements: []*knftables.Element{{
1690+
Set: nftablesAdvertisedUDNsSetV4,
1691+
Key: []string{"100.128.0.0/16"},
1692+
Comment: knftables.PtrTo[string]("test"),
1693+
}},
1694+
expectedV6Elements: []*knftables.Element{{
1695+
Set: nftablesAdvertisedUDNsSetV6,
1696+
Key: []string{"ae70::/60"},
1697+
Comment: knftables.PtrTo[string]("test"),
1698+
}},
1699+
},
1700+
{
1701+
name: "Should not add duplicate V4 and V6 entries to the for advertised network",
1702+
nad: ovntest.GenerateNAD("test", "rednad", "greenamespace",
1703+
types.Layer3Topology, "100.128.0.0/16/24,ae70::/60/64", types.NetworkRolePrimary),
1704+
isNetworkAdvertised: true,
1705+
initialElements: []*knftables.Element{
1706+
{
1707+
Set: nftablesAdvertisedUDNsSetV4,
1708+
Key: []string{"100.128.0.0/16"},
1709+
Comment: knftables.PtrTo[string]("test"),
1710+
}, {
1711+
Set: nftablesAdvertisedUDNsSetV6,
1712+
Key: []string{"ae70::/60"},
1713+
Comment: knftables.PtrTo[string]("test"),
1714+
},
1715+
},
1716+
expectedV4Elements: []*knftables.Element{{
1717+
Set: nftablesAdvertisedUDNsSetV4,
1718+
Key: []string{"100.128.0.0/16"},
1719+
Comment: knftables.PtrTo[string]("test"),
1720+
}},
1721+
expectedV6Elements: []*knftables.Element{{
1722+
Set: nftablesAdvertisedUDNsSetV6,
1723+
Key: []string{"ae70::/60"},
1724+
Comment: knftables.PtrTo[string]("test"),
1725+
}},
1726+
},
1727+
{
1728+
name: "Should remove V4 and V6 entries from the set when network for not advertised network",
1729+
nad: ovntest.GenerateNAD("test", "rednad", "greenamespace",
1730+
types.Layer3Topology, "100.128.0.0/16/24,ae70::/60/64", types.NetworkRolePrimary),
1731+
isNetworkAdvertised: false,
1732+
initialElements: []*knftables.Element{
1733+
{
1734+
Set: nftablesAdvertisedUDNsSetV4,
1735+
Key: []string{"100.128.0.0/16"},
1736+
Comment: knftables.PtrTo[string]("test"),
1737+
}, {
1738+
Set: nftablesAdvertisedUDNsSetV6,
1739+
Key: []string{"ae70::/60"},
1740+
Comment: knftables.PtrTo[string]("test"),
1741+
},
1742+
},
1743+
},
1744+
}
1745+
for _, tt := range tests {
1746+
t.Run(tt.name, func(t *testing.T) {
1747+
g := NewWithT(t)
1748+
nft := nodenft.SetFakeNFTablesHelper()
1749+
config.IPv4Mode = true
1750+
config.IPv6Mode = true
1751+
1752+
netInfo, err := util.ParseNADInfo(tt.nad)
1753+
g.Expect(err).NotTo(HaveOccurred())
1754+
1755+
err = configureAdvertisedUDNIsolationNFTables()
1756+
g.Expect(err).ToNot(HaveOccurred())
1757+
tx := nft.NewTransaction()
1758+
for _, element := range tt.initialElements {
1759+
tx.Add(element)
1760+
}
1761+
if tx.NumOperations() > 0 {
1762+
err = nft.Run(context.TODO(), tx)
1763+
g.Expect(err).NotTo(HaveOccurred())
1764+
}
1765+
udng := &UserDefinedNetworkGateway{
1766+
NetInfo: netInfo,
1767+
}
1768+
err = udng.updateAdvertisedUDNIsolationRules(tt.isNetworkAdvertised)
1769+
g.Expect(err).NotTo(HaveOccurred())
1770+
1771+
v4Elems, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV4)
1772+
g.Expect(err).NotTo(HaveOccurred())
1773+
g.Expect(v4Elems).To(HaveLen(len(tt.expectedV4Elements)))
1774+
1775+
v6Elems, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV6)
1776+
g.Expect(err).NotTo(HaveOccurred())
1777+
g.Expect(v6Elems).To(HaveLen(len(tt.expectedV6Elements)))
1778+
1779+
for i, element := range tt.expectedV4Elements {
1780+
g.Expect(element.Key).To(HaveLen(len(v4Elems[i].Key)))
1781+
g.Expect(element.Key[0]).To(BeEquivalentTo(v4Elems[i].Key[0]))
1782+
g.Expect(element.Comment).To(BeEquivalentTo(v4Elems[i].Comment))
1783+
}
1784+
for i, element := range tt.expectedV6Elements {
1785+
g.Expect(element.Key).To(HaveLen(len(v6Elems[i].Key)))
1786+
g.Expect(element.Key[0]).To(BeEquivalentTo(v6Elems[i].Key[0]))
1787+
g.Expect(element.Comment).To(BeEquivalentTo(v6Elems[i].Comment))
1788+
}
1789+
})
1790+
}
1791+
}

0 commit comments

Comments
 (0)