Skip to content

Commit

Permalink
Prevent use of one OVN network's volatile IP on another OVN network (…
Browse files Browse the repository at this point in the history
…stable-5.0) (#14405)

Since we would like to prevent the use of one OVN network's volatile IP
on another OVN network, we should consider this when retrieving external
subnet information during validation.
  • Loading branch information
tomponline authored Nov 5, 2024
2 parents 4175911 + 37c4e73 commit 7c9e679
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 25 deletions.
1 change: 1 addition & 0 deletions lxd/network/driver_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const (
subnetUsageNetworkForward
subnetUsageInstance
subnetUsageProxy
subnetUsageVolatileIP
)

// externalSubnetUsage represents usage of a subnet by a network or NIC.
Expand Down
80 changes: 55 additions & 25 deletions lxd/network/driver_ovn.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net"
"net/http"
"os"
"slices"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -4094,15 +4095,32 @@ func (n *ovn) ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink map[stri
})
}

subnetSize := 128
if keyPrefix == "ipv4" {
subnetSize = 32
}

// Find any external subnets used for network SNAT.
if netInfo.Config[fmt.Sprintf("%s.nat.address", keyPrefix)] != "" {
key := fmt.Sprintf("%s.nat.address", keyPrefix)

subnetSize := 128
if keyPrefix == "ipv4" {
subnetSize = 32
_, ipNet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", netInfo.Config[key], subnetSize))
if err != nil {
return nil, fmt.Errorf("Failed parsing %q of %q in project %q: %w", key, netInfo.Name, netProject, err)
}

externalSubnets = append(externalSubnets, externalSubnetUsage{
subnet: *ipNet,
networkProject: netProject,
networkName: netInfo.Name,
usageType: subnetUsageNetworkSNAT,
})
}

// Find the volatile IP for the network.
if netInfo.Config[fmt.Sprintf("volatile.network.%s.address", keyPrefix)] != "" {
key := fmt.Sprintf("volatile.network.%s.address", keyPrefix)

_, ipNet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", netInfo.Config[key], subnetSize))
if err != nil {
return nil, fmt.Errorf("Failed parsing %q of %q in project %q: %w", key, netInfo.Name, netProject, err)
Expand All @@ -4112,7 +4130,7 @@ func (n *ovn) ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink map[stri
subnet: *ipNet,
networkProject: netProject,
networkName: netInfo.Name,
usageType: subnetUsageNetworkSNAT,
usageType: subnetUsageVolatileIP,
})
}
}
Expand Down Expand Up @@ -4411,34 +4429,20 @@ func (n *ovn) ForwardCreate(forward api.NetworkForwardsPost, clientType request.
return err
}

externalSubnetsInUse, err := n.getExternalSubnetInUse(n.config["network"])
if err != nil {
return err
}

// Check the listen address subnet is allowed within both the uplink's external routes and any
// project restricted subnets.
err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, listenAddressNet)
if err != nil {
return err
}

// Check the listen address subnet doesn't fall within any existing OVN network external subnets.
for _, externalSubnetUser := range externalSubnetsInUse {
// Check if usage is from our own network.
if externalSubnetUser.networkProject == n.project && externalSubnetUser.networkName == n.name {
// Skip checking conflict with our own network's subnet or SNAT address.
// But do not allow other conflict with other usage types within our own network.
if externalSubnetUser.usageType == subnetUsageNetwork || externalSubnetUser.usageType == subnetUsageNetworkSNAT {
continue
}
}

if SubnetContains(&externalSubnetUser.subnet, listenAddressNet) || SubnetContains(listenAddressNet, &externalSubnetUser.subnet) {
// This error is purposefully vague so that it doesn't reveal any names of
// resources potentially outside of the network's project.
return fmt.Errorf("Forward listen address %q overlaps with another network or NIC", listenAddressNet.String())
}
isValid, err := n.checkAddressNotInUse(listenAddressNet)
if err != nil {
return err
} else if !isValid {
// This error is purposefully vague so that it doesn't reveal any names of
// resources potentially outside of the network's project.
return fmt.Errorf("Forward listen address %q overlaps with another network or NIC", listenAddressNet.String())
}

client, err := openvswitch.NewOVN(n.state)
Expand Down Expand Up @@ -5088,3 +5092,29 @@ func (n *ovn) forPeers(f func(targetOVNNet *ovn) error) error {

return nil
}

// checkAddressNotInUse checks that a given network subnet does not fall within
// any existing OVN network external subnets on the same uplink.
func (n *ovn) checkAddressNotInUse(netip *net.IPNet) (bool, error) {
externalSubnetsInUse, err := n.getExternalSubnetInUse(n.config["network"])
if err != nil {
return false, err
}

for _, externalSubnetUser := range externalSubnetsInUse {
// Check if usage is from our own network.
if externalSubnetUser.networkProject == n.project && externalSubnetUser.networkName == n.name {
// Skip checking conflict with our own network's subnet, SNAT address, or volatile IP.
// But do not allow other conflict with other usage types within our own network.
if slices.Contains([]subnetUsageType{subnetUsageNetwork, subnetUsageNetworkSNAT, subnetUsageVolatileIP}, externalSubnetUser.usageType) {
continue
}
}

if SubnetContains(&externalSubnetUser.subnet, netip) || SubnetContains(netip, &externalSubnetUser.subnet) {
return false, nil
}
}

return true, nil
}

0 comments on commit 7c9e679

Please sign in to comment.