Skip to content

Commit e6eb957

Browse files
tssuryatssurya
authored
Merge pull request #5163 from kyrtapz/bgp_isolation_e2e
Block local processes from accessing advertised UDNs
2 parents 981742d + ea10061 commit e6eb957

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)