Add validation for secstorage.allowed.internal.sites (apache#9567)

* Add validation for secstorage.allowed.internal.sites

* Address comments

* Apply suggestions from code review

Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

* Address comments

---------

Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

Author: 2 people

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

@@ -307,6 +307,8 @@
 import com.google.common.collect.Sets;
 import com.googlecode.ipv6.IPv6Network;
 
+import static com.cloud.configuration.Config.SecStorageAllowedInternalDownloadSites;
+
 public class ConfigurationManagerImpl extends ManagerBase implements ConfigurationManager, ConfigurationService, Configurable {
     public static final String PERACCOUNT = "peraccount";
     public static final String PERZONE = "perzone";
@@ -1330,6 +1332,18 @@ protected String validateConfigurationValue(final String name, String value, fin
             }
         }
 
+        if (type.equals(String.class)) {
+            if (name.equalsIgnoreCase(SecStorageAllowedInternalDownloadSites.key()) && StringUtils.isNotEmpty(value)) {
+                final String[] cidrs = value.split(",");
+                for (final String cidr : cidrs) {
+                    if (!NetUtils.isValidIp4(cidr) && !NetUtils.isValidIp6(cidr) && !NetUtils.getCleanIp4Cidr(cidr).equals(cidr)) {
+                        s_logger.error(String.format("Invalid CIDR %s value specified for the config %s", cidr, name));
+                        throw new InvalidParameterValueException(String.format("Invalid CIDR %s value specified for the config %s", cidr, name));
+                    }
+                }
+            }
+        }
+
         if (configuration == null ) {
             //range validation has to be done per case basis, for now
             //return in case of Configkey parameters

services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java

@@ -157,6 +157,8 @@
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
 
+import static com.cloud.configuration.Config.SecStorageAllowedInternalDownloadSites;
+
 /**
 * Class to manage secondary storages. <br><br>
 * Possible secondary storage VM state transition cases:<br>
@@ -399,6 +401,9 @@ private List<String> getAllowedInternalSiteCidrs() {
         String[] cidrs = _allowedInternalSites.split(",");
         for (String cidr : cidrs) {
             if (NetUtils.isValidIp4Cidr(cidr) || NetUtils.isValidIp4(cidr) || !cidr.startsWith("0.0.0.0")) {
+                if (NetUtils.getCleanIp4Cidr(cidr).equals(cidr)) {
+                    s_logger.warn(String.format("Invalid CIDR %s in %s", cidr, SecStorageAllowedInternalDownloadSites.key()));
+                }
                 allowedCidrs.add(cidr);
             }
         }

ui/src/views/setting/ConfigurationValue.vue

@@ -274,7 +274,7 @@ export default {
         this.$message.error(this.$t('message.error.save.setting'))
         this.$notification.error({
           message: this.$t('label.error'),
-          description: this.$t('message.error.save.setting')
+          description: error?.response?.data?.updateconfigurationresponse?.errortext || this.$t('message.error.save.setting')
         })
       }).finally(() => {
         this.valueLoading = false

utils/src/main/java/com/cloud/utils/net/NetUtils.java

@@ -628,6 +628,18 @@ public static String getCidrFromGatewayAndNetmask(final String gatewayStr, final
         return long2Ip(firstPart) + "/" + size;
     }
 
+    public static String getCleanIp4Cidr(final String cidr) {
+        if (!isValidIp4Cidr(cidr)) {
+            throw new CloudRuntimeException("Invalid CIDR: " + cidr);
+        }
+        String gateway = cidr.split("/")[0];
+        Long netmaskSize = Long.parseLong(cidr.split("/")[1]);
+        final long ip = ip2Long(gateway);
+        final long startNetMask = ip2Long(getCidrNetmask(netmaskSize));
+        final long start = (ip & startNetMask);
+        return String.format("%s/%s", long2Ip(start), netmaskSize);
+    }
+
     public static String[] getIpRangeFromCidr(final String cidr, final long size) {
         assert size < MAX_CIDR : "You do know this is not for ipv6 right?  Keep it smaller than 32 but you have " + size;
         final String[] result = new String[2];

utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java

@@ -296,6 +296,17 @@ public void testIsValidCIDR() throws Exception {
         assertTrue(NetUtils.isValidIp4Cidr(cidrThird));;
     }
 
+    @Test
+    public void testGetCleanIp4Cidr() throws Exception {
+        final String cidrFirst = "10.0.144.0/20";
+        final String cidrSecond = "10.0.151.5/20";
+        final String cidrThird = "10.0.144.10/21";
+
+        assertEquals(cidrFirst, NetUtils.getCleanIp4Cidr(cidrFirst));
+        assertEquals("10.0.144.0/20", NetUtils.getCleanIp4Cidr(cidrSecond));
+        assertEquals("10.0.144.0/21", NetUtils.getCleanIp4Cidr(cidrThird));;
+    }
+
     @Test
     public void testIsValidCidrList() throws Exception {
         final String cidrFirst = "10.0.144.0/20,1.2.3.4/32,5.6.7.8/24";