Certificate and VM hostname validation improvements (#10051)

* Certificate and VM hostname validation improvements

* Improve certificate name validation and some code/log improvements

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java

@@ -42,6 +42,9 @@ public final class LibvirtGetVmIpAddressCommandWrapper extends CommandWrapper<Ge
         String ip = null;
         boolean result = false;
         String vmName = command.getVmName();
+        if (!NetUtils.verifyDomainNameLabel(vmName, true)) {
+            return new Answer(command, result, ip);
+        }
         String sanitizedVmName = sanitizeBashCommandArgument(vmName);
         String networkCidr = command.getVmNetworkCidr();
         List<String[]> commands = new ArrayList<>();

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java

@@ -37,6 +37,7 @@ import com.cloud.resource.ResourceWrapper;
 import com.cloud.utils.FileUtil;
 import com.cloud.utils.PropertiesUtil;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
 import com.cloud.utils.script.Script;
 
 @ResourceWrapper(handles =  SetupDirectDownloadCertificateCommand.class)
@@ -132,6 +133,9 @@ public class LibvirtSetupDirectDownloadCertificateCommandWrapper extends Command
     public Answer execute(SetupDirectDownloadCertificateCommand cmd, LibvirtComputingResource serverResource) {
         String certificate = cmd.getCertificate();
         String certificateName = cmd.getCertificateName();
+        if (!NetUtils.verifyDomainNameLabel(certificateName, false)) {
+            return new Answer(cmd, false, "The provided certificate name is invalid");
+        }
 
         try {
             File agentFile = getAgentPropertiesFile();

plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixGetVmIpAddressCommandWrapper.java

@@ -63,20 +63,18 @@ public final class CitrixGetVmIpAddressCommandWrapper extends CommandWrapper<Get
             }
 
             if (vmIp != null) {
-                s_logger.debug("VM " +vmName + " ip address got retrieved "+vmIp);
+                s_logger.debug("VM " + vmName + " IP address got retrieved " + vmIp);
                 result = true;
                 return new Answer(command, result, vmIp);
             }
-
+        } catch (Types.XenAPIException e) {
-        }catch (Types.XenAPIException e) {
+            s_logger.debug("Got exception in GetVmIpAddressCommand " + e.getMessage());
-            s_logger.debug("Got exception in GetVmIpAddressCommand "+ e.getMessage());
+            errorMsg = "Failed to retrieve vm ip addr, exception: " + e.getMessage();
-            errorMsg = "Failed to retrived vm ip addr, exception: "+e.getMessage();
+        } catch (XmlRpcException e) {
-        }catch (XmlRpcException e) {
+            s_logger.debug("Got exception in GetVmIpAddressCommand " + e.getMessage());
-            s_logger.debug("Got exception in GetVmIpAddressCommand "+ e.getMessage());
+            errorMsg = "Failed to retrieve vm ip addr, exception: " + e.getMessage();
-            errorMsg = "Failed to retrived vm ip addr, exception: "+e.getMessage();
         }
 
         return new Answer(command, result, errorMsg);
-
     }
 }

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

@@ -751,8 +751,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
     }
 
     private class VmIpAddrFetchThread extends ManagedContextRunnable {
-
-
         long nicId;
         long vmId;
         String vmName;
@@ -775,7 +773,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             boolean decrementCount = true;
 
             try {
-                s_logger.debug("Trying for vm "+ vmId +" nic Id "+nicId +" ip retrieval ...");
+                s_logger.debug(String.format("Trying IP retrieval for VM %s (%d), nic Id %d", vmName, vmId, nicId));
                 Answer answer = _agentMgr.send(hostId, cmd);
                 NicVO nic = _nicDao.findById(nicId);
                 if (answer.getResult()) {
@@ -786,12 +784,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                         if (nic != null) {
                             nic.setIPv4Address(vmIp);
                             _nicDao.update(nicId, nic);
-                            s_logger.debug("Vm "+ vmId +" IP "+vmIp +" got retrieved successfully");
+                            s_logger.debug(String.format("VM %s (%d) - IP %s retrieved successfully", vmName, vmId, vmIp));
                             vmIdCountMap.remove(nicId);
                             decrementCount = false;
                             ActionEventUtils.onActionEvent(User.UID_SYSTEM, Account.ACCOUNT_ID_SYSTEM,
                                     Domain.ROOT_DOMAIN, EventTypes.EVENT_NETWORK_EXTERNAL_DHCP_VM_IPFETCH,
-                                    "VM " + vmId + " nic id " + nicId + " ip address " + vmIp + " got fetched successfully", vmId, ApiCommandResourceType.VirtualMachine.toString());
+                                    "VM " + vmId + ", nic id " + nicId + ", IP address " + vmIp + " fetched successfully", vmId, ApiCommandResourceType.VirtualMachine.toString());
                         }
                     }
                 } else {
@@ -802,7 +800,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                         _nicDao.update(nicId, nic);
                     }
                     if (answer.getDetails() != null) {
-                        s_logger.debug("Failed to get vm ip for Vm "+ vmId + answer.getDetails());
+                        s_logger.debug(String.format("Failed to get IP for VM %s (%d), details: %s", vmName, vmId, answer.getDetails()));
                     }
                 }
             } catch (OperationTimedoutException e) {
@@ -813,7 +811,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                 if (decrementCount) {
                     VmAndCountDetails vmAndCount = vmIdCountMap.get(nicId);
                     vmAndCount.decrementCount();
-                    s_logger.debug("Ip is not retrieved for VM " + vmId +" nic "+nicId + " ... decremented count to "+vmAndCount.getRetrievalCount());
+                    s_logger.debug(String.format("IP is not retrieved for VM %s (%d), nic %d ... decremented count to %d", vmName, vmId, nicId, vmAndCount.getRetrievalCount()));
                     vmIdCountMap.put(nicId, vmAndCount);
                 }
             }

server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java

@@ -103,6 +103,7 @@ import com.cloud.storage.dao.VMTemplatePoolDao;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
 import com.cloud.utils.security.CertificateHelper;
 
 import sun.security.x509.X509CertImpl;
@@ -471,10 +472,18 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown
     @Override
     public Pair<DirectDownloadCertificate, List<HostCertificateStatus>> uploadCertificateToHosts(
             String certificateCer, String alias, String hypervisor, Long zoneId, Long hostId) {
-        if (alias != null && (alias.equalsIgnoreCase("cloud") || alias.startsWith("cloudca"))) {
+        if (StringUtils.isBlank(alias)) {
+            throw new CloudRuntimeException("Certificate name not provided, please provide a valid name");
+        }
+
+        if (alias.equalsIgnoreCase("cloud") || alias.startsWith("cloudca")) {
             throw new CloudRuntimeException("Please provide a different alias name for the certificate");
         }
 
+        if (!NetUtils.verifyDomainNameLabel(alias, false)) {
+            throw new CloudRuntimeException("The provided certificate name is invalid, please provide a valid name");
+        }
+
         List<HostVO> hosts;
         DirectDownloadCertificateVO certificateVO;
         HypervisorType hypervisorType = HypervisorType.getType(hypervisor);

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

@@ -99,6 +99,10 @@ public class NetUtils {
     public final static int IPV6_EUI64_11TH_BYTE = -1;
     public final static int IPV6_EUI64_12TH_BYTE = -2;
 
+    // Regex
+    public final static Pattern HOSTNAME_PATTERN = Pattern.compile("[a-zA-Z0-9-]+");
+    public final static Pattern START_HOSTNAME_PATTERN = Pattern.compile("^[0-9-].*");
+
     public static String extractHost(String uri) throws URISyntaxException {
         return (new URI(uri)).getHost();
     }
@@ -1061,13 +1065,13 @@ public class NetUtils {
         if (hostName.length() > 63 || hostName.length() < 1) {
             s_logger.warn("Domain name label must be between 1 and 63 characters long");
             return false;
-        } else if (!hostName.toLowerCase().matches("[a-z0-9-]*")) {
+        } else if (!HOSTNAME_PATTERN.matcher(hostName).matches()) {
             s_logger.warn("Domain name label may contain only the ASCII letters 'a' through 'z' (in a case-insensitive manner)");
             return false;
         } else if (hostName.startsWith("-") || hostName.endsWith("-")) {
-            s_logger.warn("Domain name label can not start  with a hyphen and digit, and must not end with a hyphen");
+            s_logger.warn("Domain name label can not start or end with a hyphen");
             return false;
-        } else if (isHostName && hostName.matches("^[0-9-].*")) {
+        } else if (isHostName && START_HOSTNAME_PATTERN.matcher(hostName).matches()) {
             s_logger.warn("Host name can't start with digit");
             return false;
         }