diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index dfea381aa..dddfcdaac 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -1597,7 +1597,7 @@ class PortNetworkPolicy : public Logger::Loggable { // 'start_key' is updated as needed below. auto start_range = it->first; - // split the current entry due to partial overlap in the beginning? + // split the current entry due to partial overlap in the beginning. // For example, if the current entry is 80-8080 and we are adding 4040-9999, // the current entry should be split to two ranges 80-4039 and 4040-8080, // both of which should retain their current rules, but new rules should only be @@ -1620,6 +1620,7 @@ class PortNetworkPolicy : public Logger::Loggable { } // scan the range of the new rule, filling the gaps with new (partial) ranges + bool new_range_end_covered = false; for (; it != rules_.end() && port <= end_port && end_port >= it->first.first; it++) { auto range = it->first; // create a new entry below the current one? @@ -1644,7 +1645,7 @@ class PortNetworkPolicy : public Logger::Loggable { port = range.first; } RELEASE_ASSERT(port == range.first, "port should match the start of the current range"); - // split the current range into two due to partial overlap in the end? + // split the current range into two due to partial overlap in the end. if (end_port < range.second) { auto rules = it->second; PortRange range1 = it->first; @@ -1661,12 +1662,17 @@ class PortNetworkPolicy : public Logger::Loggable { port = end_port + 1; // one past the end break; } else { + // Current range iterator completely covers the range to insert at the end. + if (range.second == end_port) { + new_range_end_covered = true; + break; + } // current entry completely covered by the new range, skip to the next port = range.second + 1; } } - // create a new entry covering the end? - if (port <= end_port) { + // create a new entry covering the end(if not already covered). + if (!new_range_end_covered && port <= end_port) { auto new_range = std::make_pair(port, end_port); auto new_pair = rules_.emplace(new_range, PortNetworkPolicyRules{}); RELEASE_ASSERT(new_pair.second, diff --git a/tests/cilium_network_policy_test.cc b/tests/cilium_network_policy_test.cc index b74055409..2e4d42d22 100644 --- a/tests/cilium_network_policy_test.cc +++ b/tests/cilium_network_policy_test.cc @@ -511,6 +511,62 @@ TEST_F(CiliumNetworkPolicyTest, OverlappingPortRanges) { // No egress is allowed: EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 8080)); + + // Validate no overflow when splitting port ranges with existing port range containing U16 max. + EXPECT_NO_THROW(updateFromYaml(R"EOF(version_info: "1" +resources: +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: + - "10.1.2.3" + endpoint_id: 42 + ingress_per_port_policies: + - port: 32768 + end_port: 65534 + rules: + - remote_policies: [ 43 ] + - port: 32768 + end_port: 65535 + rules: + - remote_policies: [ 44 ] + - port: 1024 + end_port: 65535 + rules: + - remote_policies: [ 45 ] +)EOF")); + + expected = R"EOF(ingress: + rules: + [1024-32767]: + - rules: + - remotes: [45] + [32768-65534]: + - rules: + - remotes: [43] + - remotes: [44] + - remotes: [45] + [65535-65535]: + - rules: + - remotes: [44] + - remotes: [45] +egress: + rules: [] +)EOF"; + + EXPECT_TRUE(validate("10.1.2.3", expected)); + + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 40000)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 65535)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 40000)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 65535)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 45, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 45, 40000)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 45, 65535)); + + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 40000)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 65535)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 45, 8080)); } TEST_F(CiliumNetworkPolicyTest, DuplicatePorts) {