common/pick_address: Add IPv6 support to is_addr_in_subnet

Updated the is_addr_in_subnet function to work with both
IPv4 and IPv6 addresses. Previously, it only supported IPv4,
which caused failures when IPv6 addresses were passed in.

Changes:
 - Use inet_pton to detect IPv4 (AF_INET) or IPv6 (AF_INET6).
 - Added sockaddr_in6 for IPv6 handling while keeping sockaddr_in for IPv4.
 - Adjust the family and ifa_addr dynamically based on the address type.

Fixes: https://tracker.ceph.com/issues/67517
Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>
(cherry picked from commit d68857c1e57e93a68d9301b3beff7e652f327a9e)

diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc
index c92f2c4bc0236a112202685a314681fd4a2f4211..85aaf9b1b3a662037cff9cf7dab4c67c24c30dff 100644 (file)
--- a/src/common/pick_address.cc
+++ b/src/common/pick_address.cc
@@ -634,17 +634,24 @@ int get_iface_numa_node(
 bool is_addr_in_subnet(
   CephContext *cct,
   const std::string &networks,
-  const std::string &addr)
+  const entity_addr_t &addr)
 {
   const auto nets = get_str_list(networks);
   ceph_assert(!nets.empty());
-
   unsigned ipv = CEPH_PICK_ADDRESS_IPV4;
-  struct sockaddr_in public_addr;
-  public_addr.sin_family = AF_INET;
-
-  if(inet_pton(AF_INET, addr.c_str(), &public_addr.sin_addr) != 1) {
-    lderr(cct) << "unable to convert chosen address to string: " << addr << dendl;
+  struct sockaddr_in6 public_addr6;
+  struct sockaddr_in public_addr4;
+
+  if (addr.is_ipv4() &&
+      inet_pton(AF_INET, addr.ip_only_to_str().c_str(), &public_addr4.sin_addr) == 1) {
+    public_addr4.sin_family = AF_INET;
+  } else if (addr.is_ipv6() &&
+      inet_pton(AF_INET6, addr.ip_only_to_str().c_str(), &public_addr6.sin6_addr) == 1) {
+    public_addr6.sin6_family = AF_INET6;
+    ipv = CEPH_PICK_ADDRESS_IPV6;
+  } else {
+    std::string_view addr_type = addr.is_ipv4() ? "IPv4" : "IPv6";
+    lderr(cct) << "IP address " << addr << " is not parseable as " << addr_type << dendl;
     return false;
   }
 
@@ -652,10 +659,16 @@ bool is_addr_in_subnet(
     struct ifaddrs ifa;
     memset(&ifa, 0, sizeof(ifa));
     ifa.ifa_next = nullptr;
-    ifa.ifa_addr = (struct sockaddr*)&public_addr;
+    if (addr.is_ipv4()) {
+      ifa.ifa_addr = (struct sockaddr*)&public_addr4;
+    } else if (addr.is_ipv6()) {
+      ifa.ifa_addr = (struct sockaddr*)&public_addr6;
+    }
+
     if(matches_with_net(cct, ifa, net, ipv)) {
       return true;
     }
   }
+  lderr(cct) << "address " << addr << " is not in networks '" << networks << "'" << dendl;
   return false;
 }

diff --git a/src/common/pick_address.h b/src/common/pick_address.h
index 40575d7d1551185389f1197deb7dd90b89525909..c28a6037ded797e18661288d5b4ee4087fc71d20 100644 (file)
--- a/src/common/pick_address.h
+++ b/src/common/pick_address.h
@@ -98,6 +98,6 @@ int get_iface_numa_node(
 bool is_addr_in_subnet(
   CephContext *cct,
   const std::string &networks,
-  const std::string &addr);
+  const entity_addr_t &addr);
 
 #endif

diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc
index 6a8c31655819842b7c2d93c9633fd567eda601f0..409a61913396143249338b337c1b8607f8a3aa00 100644 (file)
--- a/src/osd/OSDMap.cc
+++ b/src/osd/OSDMap.cc
@@ -1642,12 +1642,10 @@ void OSDMap::get_out_of_subnet_osd_counts(CephContext *cct,
   for (int i = 0; i < max_osd; i++) {
     if (exists(i) && is_up(i)) {
       if (const auto& addrs = get_addrs(i).v; addrs.size() >= 2) {
-        auto v1_addr = addrs[0].ip_only_to_str();
-        if (!is_addr_in_subnet(cct, public_network, v1_addr)) {
+        if (!is_addr_in_subnet(cct, public_network, addrs[0])) {
           unreachable->emplace(i);
         }
-        auto v2_addr = addrs[1].ip_only_to_str();
-        if (!is_addr_in_subnet(cct, public_network, v2_addr)) {
+        if (!is_addr_in_subnet(cct, public_network, addrs[1])) {
           unreachable->emplace(i);
         }
       }

diff --git a/src/test/test_ipaddr.cc b/src/test/test_ipaddr.cc
index bc8dbef70d7e1a9406a53dd8f644a0d8a4cffdce..30e7254de76b0a43c583f1b33f62c00bc572a150 100644 (file)
--- a/src/test/test_ipaddr.cc
+++ b/src/test/test_ipaddr.cc
@@ -995,3 +995,158 @@ TEST(pick_address, ipv4_ipv6_enabled2)
     ASSERT_EQ(-1, r);
   }
 }
+
+// Test for IPv4 address
+TEST(is_addr_in_subnet, ipv4)
+{
+  std::string public_network = "10.1.1.0/24";
+  entity_addr_t addr;
+  addr.parse("10.1.1.2", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv4", "true");
+  cct->_conf.set_val("ms_bind_ipv6", "false");
+
+  bool r = is_addr_in_subnet(cct.get(), public_network, addr);
+  ASSERT_EQ(true, r);
+}
+
+// Test for IPv6 address
+TEST(is_addr_in_subnet, ipv6)
+{
+  std::string public_network = "2001:db8::/64";
+  entity_addr_t addr;
+  addr.parse("2001:db8::1", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv6", "true");
+  cct->_conf.set_val("ms_bind_ipv4", "false");
+
+  bool r = is_addr_in_subnet(cct.get(), public_network, addr);
+  ASSERT_EQ(true, r);
+}
+
+// Test for invalid address
+TEST(is_addr_in_subnet, invalid_address)
+{
+  std::string public_network = "10.1.1.0/24";
+  entity_addr_t addr;
+  addr.parse("192.168.1.1", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv4", "true");
+  cct->_conf.set_val("ms_bind_ipv6", "false");
+
+  bool r = is_addr_in_subnet(cct.get(), public_network, addr);
+  ASSERT_EQ(false, r);
+}
+
+// Test for malformed address
+TEST(is_addr_in_subnet, malformed_address)
+{
+  std::string public_network = "10.1.1.0/24";
+  entity_addr_t addr;
+  addr.parse("invalid_address", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv4", "true");
+  cct->_conf.set_val("ms_bind_ipv6", "false");
+
+  // Test with a malformed address
+  bool r = is_addr_in_subnet(cct.get(), public_network, addr);
+  ASSERT_EQ(false, r);
+}
+
+TEST(is_addr_in_subnet, boundary_ipv4)
+{
+  std::string public_network = "10.1.1.0/24";
+  entity_addr_t addr_low;
+  addr_low.parse("10.1.1.0", nullptr);
+  entity_addr_t addr_high;
+  addr_high.parse("10.1.1.255", nullptr);
+  entity_addr_t addr_out;
+  addr_out.parse("10.1.2.0", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv4", "true");
+  cct->_conf.set_val("ms_bind_ipv6", "false");
+
+  ASSERT_TRUE(is_addr_in_subnet(cct.get(), public_network, addr_low));
+  ASSERT_TRUE(is_addr_in_subnet(cct.get(), public_network, addr_high));
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network, addr_out));
+}
+
+TEST(is_addr_in_subnet, boundary_ipv6)
+{
+  std::string public_network = "2001:db8::/64";
+  entity_addr_t addr_low;
+  addr_low.parse("2001:db8::", nullptr);
+  entity_addr_t addr_high;
+  addr_high.parse("2001:db8:0:0:ffff:ffff:ffff:ffff", nullptr);
+  entity_addr_t addr_out;
+  addr_out.parse("2001:db9::", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv6", "true");
+  cct->_conf.set_val("ms_bind_ipv4", "false");
+
+  ASSERT_TRUE(is_addr_in_subnet(cct.get(), public_network, addr_low));
+  ASSERT_TRUE(is_addr_in_subnet(cct.get(), public_network, addr_high));
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network, addr_out));
+}
+
+TEST(is_addr_in_subnet, overlapping_subnets)
+{
+  std::string public_network_1 = "10.1.1.0/24";
+  std::string public_network_2 = "10.1.2.0/24";
+  entity_addr_t addr;
+  addr.parse("10.1.1.5", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv4", "true");
+  cct->_conf.set_val("ms_bind_ipv6", "false");
+
+  ASSERT_TRUE(is_addr_in_subnet(cct.get(), public_network_1, addr));
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network_2, addr));
+}
+
+TEST(is_addr_in_subnet, mismatched_family)
+{
+  std::string public_network_1 = "2001:db8::/64";
+  entity_addr_t addr_1;
+  addr_1.parse("10.1.1.5", nullptr);
+  
+  std::string public_network_2 = "10.1.1.0/24";
+  entity_addr_t addr_2;
+  addr_2.parse("2001:db8::1", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+  cct->_conf.set_val("ms_bind_ipv4", "true");
+  cct->_conf.set_val("ms_bind_ipv6", "true");
+
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network_1, addr_1));
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network_2, addr_2));
+}
+
+TEST(is_addr_in_subnet, invalid_subnets)
+{
+  std::string public_network_1 = "10.1.1.0/33";
+  std::string public_network_2 = "25.0.0.99/10";
+  entity_addr_t addr;
+  addr.parse("10.1.1.2", nullptr);
+
+  boost::intrusive_ptr<CephContext> cct(new CephContext(CEPH_ENTITY_TYPE_OSD), false);
+  cct->_conf._clear_safe_to_start_threads();
+
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network_1, addr)); // Invalid prefix
+  ASSERT_FALSE(is_addr_in_subnet(cct.get(), public_network_2, addr)); // Invalid subnet string
+}
+