Accept IPv6 entries in the usage exemption setting

Author: MitchDrage

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -1583,38 +1583,39 @@ protected String validateValueRange(String name, String value, Class<?> type, Co
     }
 
     /**
-     * Validates configuration values for the given name, value, and type.
-     * <ul>
-     *   <li>The value must be a comma-separated list of key-value pairs, where each value must be a positive integer.</li>
-     *   <li>Each key-value pair must be in the format "command=value", with the value being a positive integer greater than 0,
-     *          otherwise fails with an error message</li>
-     *   <li>Throws an {@link InvalidParameterValueException} if validation fails.</li>
-     * </ul>
-     *
-     * @param name  the configuration name
-     * @param value the configuration value as a comma-separated string of key-value pairs
-     * @param type  the configuration type, expected to be String
-     * @throws InvalidParameterValueException if validation fails with a specific error message
-     */
-    /**
-     * Checks that each entry is an IPv4 address or CIDR. We refuse /0 because it would exempt all traffic.
+     * Checks that each entry is an IPv4 or IPv6 address or CIDR. We refuse /0 because it would exempt all traffic.
      */
     protected String validateUsageExemptCidrs(String name, String value) {
         for (String entry : value.split(",")) {
             entry = entry.trim();
-            if (entry.isEmpty() || NetUtils.isValidIp4(entry)) {
+            if (entry.isEmpty()) {
                 continue;
             }
-            if (!NetUtils.isValidIp4Cidr(entry)) {
-                return String.format("[%s] is not a valid IPv4 address or CIDR for %s.", entry, name);
+            final String cidr = VirtualNetworkApplianceManager.normaliseUsageExemptCidr(entry);
+            if (cidr == null) {
+                return String.format("[%s] is not a valid IP address or CIDR for %s.", entry, name);
             }
-            if (NetUtils.getCidr(entry).second() == 0) {
+            if (cidr.endsWith("/0")) {
                 return String.format("[%s] would exempt all traffic, so it can't be used for %s.", entry, name);
             }
         }
         return null;
     }
 
+    /**
+     * Validates configuration values for the given name, value, and type.
+     * <ul>
+     *   <li>The value must be a comma-separated list of key-value pairs, where each value must be a positive integer.</li>
+     *   <li>Each key-value pair must be in the format "command=value", with the value being a positive integer greater than 0,
+     *          otherwise fails with an error message</li>
+     *   <li>Throws an {@link InvalidParameterValueException} if validation fails.</li>
+     * </ul>
+     *
+     * @param name  the configuration name
+     * @param value the configuration value as a comma-separated string of key-value pairs
+     * @param type  the configuration type, expected to be String
+     * @throws InvalidParameterValueException if validation fails with a specific error message
+     */
     protected void validateSpecificConfigurationValues(String name, String value, Class<?> type) {
         if (type.equals(String.class)) {
             if (name.equals(AgentManager.GranularWaitTimeForCommands.toString())) {

server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java

@@ -20,6 +20,8 @@
 
 import org.apache.cloudstack.framework.config.ConfigKey;
 
+import com.googlecode.ipv6.IPv6Network;
+
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.ResourceUnavailableException;
@@ -29,6 +31,7 @@
 import com.cloud.user.Account;
 import com.cloud.user.User;
 import com.cloud.utils.component.Manager;
+import com.cloud.utils.net.NetUtils;
 import com.cloud.vm.DomainRouterVO;
 
 /**
@@ -82,10 +85,31 @@ public interface VirtualNetworkApplianceManager extends Manager, VirtualNetworkA
     ConfigKey<Boolean> ExposeDnsAndBootpServer = new ConfigKey<>(Boolean.class, "expose.dns.externally", "Advanced", "false",
             "open dns, dhcp and bootp on the public interface", true, ConfigKey.Scope.Zone, null);
     ConfigKey<String> NetworkUsageExemptCidrs = new ConfigKey<>(String.class, "network.usage.exempt.cidrs", "Usage", "",
-            "Comma-separated IPv4 addresses or CIDRs. Traffic between guest networks and these addresses doesn't count " +
-            "towards virtual router network usage. Routers pick up a change the next time they start or restart.",
+            "Comma-separated IPv4 or IPv6 addresses or CIDRs. Traffic between guest networks and these addresses doesn't count " +
+            "towards virtual router network usage. Virtual routers only count IPv4 traffic, so they ignore IPv6 entries. " +
+            "Routers pick up a change the next time they start or restart.",
             true, ConfigKey.Scope.Zone, null, null, null, null, null, ConfigKey.Kind.CSV, null);
 
+    /**
+     * Tidies one entry of {@link #NetworkUsageExemptCidrs} into a CIDR, or returns null if it isn't a valid address or CIDR.
+     * For example, 192.0.2.5 becomes 192.0.2.5/32, 198.51.100.70/24 becomes 198.51.100.0/24 and 2001:db8::5 becomes
+     * 2001:db8::5/128. Callers refuse a /0 result, as it would exempt all traffic.
+     */
+    static String normaliseUsageExemptCidr(String entry) {
+        if (NetUtils.isValidIp4(entry)) {
+            entry = entry + "/32";
+        } else if (NetUtils.isValidIp6(entry)) {
+            entry = entry + "/128";
+        }
+        if (NetUtils.isValidIp4Cidr(entry)) {
+            return NetUtils.getCidrSubNet(entry) + "/" + NetUtils.getCidr(entry).second();
+        }
+        if (entry.contains(":") && NetUtils.isValidIp6Cidr(entry)) {
+            return IPv6Network.fromString(entry).toString();
+        }
+        return null;
+    }
+
     ConfigKey<String> VirtualRouterServiceOffering = new ConfigKey<>(String.class, VirtualRouterServiceOfferingCK, "Advanced", "",
             "Uuid of the service offering used by virtual routers; if NULL - system offering will be used", true, ConfigKey.Scope.Account, null);
 

server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java

@@ -3373,8 +3373,9 @@ public String getConfigComponentName() {
     }
 
     /**
-     * Turns {@link #NetworkUsageExemptCidrs} into the list we pass to the router. For example, 192.0.2.5 becomes
-     * 192.0.2.5/32 and 198.51.100.70/24 becomes 198.51.100.0/24. Bad entries and /0 are logged and left out.
+     * Turns {@link #NetworkUsageExemptCidrs} into the list we pass to the router, tidied by
+     * {@link VirtualNetworkApplianceManager#normaliseUsageExemptCidr}. IPv6 entries are passed on too.
+     * The router ignores them for now. Bad entries and /0 are logged and left out.
      */
     protected List<String> parseUsageExemptCidrs(final String value) {
         final List<String> cidrs = new ArrayList<>();
@@ -3386,14 +3387,11 @@ protected List<String> parseUsageExemptCidrs(final String value) {
             if (entry.isEmpty()) {
                 continue;
             }
-            if (NetUtils.isValidIp4(entry)) {
-                entry = entry + "/32";
-            }
-            if (!NetUtils.isValidIp4Cidr(entry) || NetUtils.getCidr(entry).second() == 0) {
-                logger.warn("Skipping [{}] in {}: not a valid IPv4 address or CIDR, or it's /0", entry, NetworkUsageExemptCidrs.key());
+            final String cidr = VirtualNetworkApplianceManager.normaliseUsageExemptCidr(entry);
+            if (cidr == null || cidr.endsWith("/0")) {
+                logger.warn("Skipping [{}] in {}: not a valid IP address or CIDR, or it's /0", entry, NetworkUsageExemptCidrs.key());
                 continue;
             }
-            final String cidr = NetUtils.getCidrSubNet(entry) + "/" + NetUtils.getCidr(entry).second();
             if (!cidrs.contains(cidr)) {
                 cidrs.add(cidr);
             }

server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java

@@ -870,18 +870,20 @@ private String validateUsageExemptCidrs(String value) {
     public void validateValueRangeTestAcceptsUsageExemptAddressesAndCidrs() {
         Assert.assertNull(validateUsageExemptCidrs(""));
         Assert.assertNull(validateUsageExemptCidrs("192.0.2.5, 198.51.100.70/24,,203.0.113.0/24"));
+        Assert.assertNull(validateUsageExemptCidrs("2001:db8::5,2001:db8:0:1::7/64"));
     }
 
     @Test
     public void validateValueRangeTestRejectsInvalidUsageExemptEntry() {
         Assert.assertNotNull(validateUsageExemptCidrs("192.0.2.5,bogus"));
         Assert.assertNotNull(validateUsageExemptCidrs("192.0.2.300"));
-        Assert.assertNotNull(validateUsageExemptCidrs("2001:db8::/32"));
+        Assert.assertNotNull(validateUsageExemptCidrs("2001:db8::/129"));
     }
 
     @Test
     public void validateValueRangeTestRejectsUsageExemptMatchAll() {
         Assert.assertNotNull(validateUsageExemptCidrs("0.0.0.0/0"));
+        Assert.assertNotNull(validateUsageExemptCidrs("::/0"));
     }
 
     @Test

server/src/test/java/com/cloud/network/router/VirtualNetworkApplianceManagerImplTest.java

@@ -428,9 +428,15 @@ public void parseUsageExemptCidrsTestNormalisesEntries() {
         Assert.assertEquals(List.of("192.0.2.5/32", "198.51.100.0/24"), cidrs);
     }
 
+    @Test
+    public void parseUsageExemptCidrsTestNormalisesIpv6Entries() {
+        List<String> cidrs = virtualNetworkApplianceManagerImpl.parseUsageExemptCidrs("2001:db8::5, 2001:db8:0:1::7/64,192.0.2.5");
+        Assert.assertEquals(List.of("2001:db8::5/128", "2001:db8:0:1::/64", "192.0.2.5/32"), cidrs);
+    }
+
     @Test
     public void parseUsageExemptCidrsTestSkipsInvalidEntries() {
-        List<String> cidrs = virtualNetworkApplianceManagerImpl.parseUsageExemptCidrs("not-an-ip,0.0.0.0/0,10.0.0.0/33,192.168.1.300,172.16.0.0/12");
+        List<String> cidrs = virtualNetworkApplianceManagerImpl.parseUsageExemptCidrs("not-an-ip,0.0.0.0/0,::/0,10.0.0.0/33,192.168.1.300,172.16.0.0/12");
         Assert.assertEquals(List.of("172.16.0.0/12"), cidrs);
     }
 }

systemvm/debian/opt/cloud/bin/cs/CsConfig.py

@@ -116,17 +116,20 @@ def expose_dns(self):
         return self.cmdline().idata().get('exposedns', 'false') == 'true'
 
     def get_usage_exempt_cidrs(self):
-        """ The CIDRs to leave out of network usage, tidied to match how iptables-save prints them.
-        Without that, each run would see the rules as changed and rewrite them. """
+        """ The IPv4 CIDRs to leave out of network usage, tidied to match how iptables-save prints them.
+        Without that, each run would see the rules as changed and rewrite them.
+        IPv6 entries are skipped, as the router only counts IPv4 traffic. """
         cidrs = []
         for value in self.cmdline().idata().get('usage_exempt_cidrs', '').split(','):
             value = value.strip()
             if not value:
                 continue
             try:
-                network = ipaddress.IPv4Network(value, strict=False)
+                network = ipaddress.ip_network(value, strict=False)
             except ValueError:
-                logging.warning("Skipping %s in usage exempt CIDRs: not a valid IPv4 address or CIDR", value)
+                logging.warning("Skipping %s in usage exempt CIDRs: not a valid IP address or CIDR", value)
+                continue
+            if network.version != 4:
                 continue
             if network.prefixlen == 0:
                 logging.warning("Skipping %s in usage exempt CIDRs: it would exempt all traffic", value)

systemvm/test/TestCsConfig.py

@@ -47,6 +47,11 @@ def test_usage_exempt_cidrs_skip_invalid_and_match_all(self):
         cidrs = self.usage_exempt_cidrs({"usage_exempt_cidrs": "not-an-ip,0.0.0.0/0,192.168.1.300,172.16.0.0/12"})
         self.assertEqual(cidrs, ["172.16.0.0/12"])
 
+    def test_usage_exempt_cidrs_skip_ipv6(self):
+        # the router only counts IPv4 traffic, so IPv6 entries have nothing to exempt
+        cidrs = self.usage_exempt_cidrs({"usage_exempt_cidrs": "2001:db8::5/128,192.0.2.5/32,2001:db8:0:1::/64"})
+        self.assertEqual(cidrs, ["192.0.2.5/32"])
+
 
 if __name__ == '__main__':
     unittest.main()