Modify validation logic to use white list instead of blacklist

Author: dhlaluku

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -526,14 +526,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
     private static final ConfigKey<Boolean> EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class,
             "enable.additional.vm.configuration", "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account);
 
-    private static final ConfigKey<String> KvmAdditionalConfigBlackList = new ConfigKey<>("Advanced", String.class,
-            "additional.vm.configuration.black.list.kvm", "name, uuid, memory, currentMemory", "Comma separated list of disallowed additional configuration options.", true);
+    private static final ConfigKey<String> KvmAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class,
+            "allow.additional.vm.configuration.list.kvm", "memoryBacking", "Comma separated list of allowed additional configuration options.", true);
 
-    private static final ConfigKey<String> XenServerAdditionalConfigBlackList = new ConfigKey<>("Advanced", String.class,
-            "additional.vm.configuration.black.list.xenserver", "is-a-template, memory-static-max, memory-dynamic-max, memory-dynamic-min, memory-static-min", "Comma separated list of disallowed additional configuration options", true);
+    private static final ConfigKey<String> XenServerAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class,
+            "allow.additional.vm.configuration.list.xenserver", "HVM-boot-policy, PV-bootloader, PV-args", "Comma separated list of allowed additional configuration options", true);
 
-    private static final ConfigKey<String> VmwareAdditionalConfigBlackList = new ConfigKey<>("Advanced", String.class,
-            "additional.vm.configuration.black.list.vmware", "guestOS, displayName, virtualHW.version, migrate.hostLog, nvram", "Comma separated list of disallowed additional configuration options.", true);
+    private static final ConfigKey<String> VmwareAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class,
+            "allow.additional.vm.configuration.list.vmware", "hypervisor.cpuid.v0", "Comma separated list of allowed additional configuration options.", true);
 
     private static final ConfigKey<Boolean> VmDestroyForcestop = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.destroy.forcestop", "false",
             "On destroy, force-stop takes this value ", true);
@@ -4924,28 +4924,26 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE
 
     /**
      * Persist extra configuration data in the user_vm_details table as key/value pair
-     *
      * @param decodedUrl String consisting of the extra config data to appended onto the vmx file for VMware instances
-     * @param vm
      */
     protected void persistExtraConfigVmware(String decodedUrl, UserVm vm) {
         boolean isValidConfig = isValidKeyValuePair(decodedUrl);
         if (isValidConfig) {
             String[] extraConfigs = decodedUrl.split("\\r?\\n");
             for (String cfg : extraConfigs) {
                 // Validate cfg against unsupported operations set by admin here
-                String[] blackList = VmwareAdditionalConfigBlackList.value().split(",");
-                boolean isValidAgainstBlacklistedCfg = isValidXenOrVmwareConfiguration(cfg, blackList);
+                String[] allowedKeyList = VmwareAdditionalConfigAllowList.value().split(",");
+                boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList);
                 String[] paramArray = cfg.split("=");
-                if (isValidAgainstBlacklistedCfg && paramArray.length == 2) {
+                if (validXenOrVmwareConfiguration && paramArray.length == 2) {
                     try {
                         userVmDetailsDao.addDetail(vm.getId(), paramArray[0].trim(), paramArray[1].trim(), true);
                     } catch (ArrayIndexOutOfBoundsException e) {
                         throw new CloudRuntimeException("Issue occurred during parsing of:" + cfg);
                     }
 
                 } else {
-                    throw new CloudRuntimeException("Extra config " + cfg + " contains a blacklisted key operations by Root admin");
+                    throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for VMware hypervisor hosts.");
                 }
             }
         } else {
@@ -4960,7 +4958,6 @@ protected void persistExtraConfigVmware(String decodedUrl, UserVm vm) {
      *
      * @param decodedUrl A string containing extra configuration settings as key/value pairs seprated by newline escape character
      *                   e.x PV-bootloader=pygrub\nPV-args=console\nHV-Boot-policy=""
-     * @param vm
      */
     protected void persistExtraConfigXenServer(String decodedUrl, UserVm vm) {
         boolean isValidConfig = isValidKeyValuePair(decodedUrl);
@@ -4970,9 +4967,9 @@ protected void persistExtraConfigXenServer(String decodedUrl, UserVm vm) {
             String extraConfigKey = ApiConstants.EXTRA_CONFIG + "-";
             for (String cfg : extraConfigs) {
                 // Validate cfg against unsupported operations set by admin here
-                String[] blackList = XenServerAdditionalConfigBlackList.value().split(",");
-                boolean isValidAgainstBlacklistedCfg = isValidXenOrVmwareConfiguration(cfg, blackList);
-                if (isValidAgainstBlacklistedCfg) {
+                String[] allowedKeyList = XenServerAdditionalConfigAllowList.value().split(",");
+                boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList);
+                if (validXenOrVmwareConfiguration) {
                     userVmDetailsDao.addDetail(vm.getId(), extraConfigKey + String.valueOf(i), cfg, true);
                     i++;
                 } else {
@@ -5011,29 +5008,34 @@ protected boolean isValidKeyValuePair(String decodedUrl) {
         return matcher.matches();
     }
 
-    protected boolean isValidXenOrVmwareConfiguration(String cfg, String[] blackList) {
+    /**
+     * Validates key/value pair strings passed as extra configuration for XenServer and Vmware
+     * @param cfg configuration key-value pair
+     * @param allowedKeyList list of allowed configuration keys for XenServer and VMware
+     * @return
+     */
+    protected boolean isValidXenOrVmwareConfiguration(String cfg, String[] allowedKeyList) {
         // This should be of minimum length 1
         // Value is ignored in case it is empty
         String[] cfgKeyValuePair = cfg.split("=");
         if (cfgKeyValuePair.length >= 1) {
-            for (String diallowedKey : blackList) {
-                if (cfgKeyValuePair[0].equalsIgnoreCase(diallowedKey.trim())) {
-                    return false;
+            for (String allowedKey : allowedKeyList) {
+                if (cfgKeyValuePair[0].equalsIgnoreCase(allowedKey.trim())) {
+                    return true;
                 }
             }
         } else {
             String msg = String.format("An incorrect configuration %s has been passed", cfg);
             throw new CloudRuntimeException(msg);
         }
-        return true;
+        return false;
     }
 
     /**
      * Persist extra configuration data on KVM
      * persisted in the user_vm_details DB as extraconfig-1, and so on depending on the number of configurations
      * For KVM, extra config is passed as XML
-     *
-     * @param decodedUrl
+     * @param decodedUrl string containing xml configuration to be persisted into user_vm_details table
      * @param vm
      */
     protected void persistExtraConfigKvm(String decodedUrl, UserVm vm) {
@@ -5061,24 +5063,23 @@ protected void persistExtraConfigKvm(String decodedUrl, UserVm vm) {
      * This method is called by the persistExtraConfigKvm
      * Validates passed extra configuration data for KVM and validates against blacklist of unwanted commands
      * controlled by Root admin
-     *
-     * @param decodedUrl
+     * @param decodedUrl string containing xml configuration to be validated
      */
     protected void validateKvmExtraConfig(String decodedUrl) {
-        String[] configOptionBlackList = KvmAdditionalConfigBlackList.value().split(",");
+        String[] allowedConfigOptionList = KvmAdditionalConfigAllowList.value().split(",");
         String msg = "An invalid extra configuration option has been supplied: ";
-        // Skip blacklist validation for DPDK
+        // Skip allowed keys validation validation for DPDK
         if (!decodedUrl.contains(":")) {
             try {
                 DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
                 InputSource src = new InputSource();
                 src.setCharacterStream(new StringReader(String.format("<config>\n%s\n</config>", decodedUrl)));
                 Document doc = builder.parse(src);
                 doc.getDocumentElement().normalize();
-                for (String tag : configOptionBlackList) {
+                for (String tag : allowedConfigOptionList) {
                     NodeList nodeList = doc.getElementsByTagName(tag.trim());
-                    // Node list should be empty to show blacklisted command is not contained in passed XML
-                    if (nodeList.getLength() != 0) {
+                    // Node list should not be empty to show that allowed command is contained in passed XML
+                    if (nodeList.getLength() == 0) {
                         throw new CloudRuntimeException(msg + tag);
                     }
                 }
@@ -5090,8 +5091,6 @@ protected void validateKvmExtraConfig(String decodedUrl) {
 
     /**
      * Adds extra config data to guest VM instances
-     *
-     * @param vm
      * @param extraConfig Extra Configuration settings to be added in UserVm instances for KVM, XenServer and VMware
      */
     protected void addExtraConfig(UserVm vm, String extraConfig) {
@@ -6673,8 +6672,8 @@ public String getConfigComponentName() {
     @Override
     public ConfigKey<?>[] getConfigKeys() {
         return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax,
-            VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, KvmAdditionalConfigBlackList,
-                XenServerAdditionalConfigBlackList, VmwareAdditionalConfigBlackList};
+            VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, KvmAdditionalConfigAllowList,
+                XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList};
     }
 
     @Override