From 8c2a0161e5a8d5da0880b814a8debd8b03b3e491 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 5 Dec 2024 18:35:35 +0530 Subject: [PATCH 1/2] Certificate and VM hostname validation improvements --- .../wrapper/LibvirtGetVmIpAddressCommandWrapper.java | 3 +++ ...bvirtSetupDirectDownloadCertificateCommandWrapper.java | 4 ++++ utils/src/main/java/com/cloud/utils/net/NetUtils.java | 8 ++++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java index da2839d9cee9..d65b6907eeb5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java @@ -42,6 +42,9 @@ public Answer execute(final GetVmIpAddressCommand command, final LibvirtComputin 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 commands = new ArrayList<>(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java index 0774d306b8a1..d2b69412a728 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java @@ -37,6 +37,7 @@ 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 @@ protected void cleanupTemporaryFile(String temporaryFile) { 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(); diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index 1b4ebcccf94d..78af875a3be9 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/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 static boolean verifyDomainNameLabel(final String hostName, final boolean 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"); 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; } From e0c707cd3780c98083d473702702453f9a40fdf4 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 18 Dec 2024 16:45:19 +0530 Subject: [PATCH 2/2] Improve certificate name validation and some code/log improvements --- .../CitrixGetVmIpAddressCommandWrapper.java | 16 +++++++--------- .../java/com/cloud/vm/UserVmManagerImpl.java | 12 +++++------- .../download/DirectDownloadManagerImpl.java | 11 ++++++++++- .../main/java/com/cloud/utils/net/NetUtils.java | 2 +- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixGetVmIpAddressCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixGetVmIpAddressCommandWrapper.java index b67ef0850baf..e03708faf860 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixGetVmIpAddressCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixGetVmIpAddressCommandWrapper.java @@ -63,20 +63,18 @@ public Answer execute(final GetVmIpAddressCommand command, final CitrixResourceB } 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) { - s_logger.debug("Got exception in GetVmIpAddressCommand "+ e.getMessage()); - errorMsg = "Failed to retrived vm ip addr, exception: "+e.getMessage(); - }catch (XmlRpcException e) { - s_logger.debug("Got exception in GetVmIpAddressCommand "+ e.getMessage()); - errorMsg = "Failed to retrived vm ip addr, exception: "+e.getMessage(); + } catch (Types.XenAPIException e) { + s_logger.debug("Got exception in GetVmIpAddressCommand " + e.getMessage()); + errorMsg = "Failed to retrieve vm ip addr, exception: " + e.getMessage(); + } catch (XmlRpcException e) { + s_logger.debug("Got exception in GetVmIpAddressCommand " + e.getMessage()); + errorMsg = "Failed to retrieve vm ip addr, exception: " + e.getMessage(); } return new Answer(command, result, errorMsg); - } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 6e75512948a3..94034da4c8fb 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -751,8 +751,6 @@ public void decrementCount() { } private class VmIpAddrFetchThread extends ManagedContextRunnable { - - long nicId; long vmId; String vmName; @@ -775,7 +773,7 @@ protected void runInContext() { 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 @@ protected void runInContext() { 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 @@ protected void runInContext() { _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 @@ protected void runInContext() { 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); } } diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 0a21815789d1..99af3068f074 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -103,6 +103,7 @@ 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 @@ protected void certificateSanity(String certificatePem) { @Override public Pair> 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 hosts; DirectDownloadCertificateVO certificateVO; HypervisorType hypervisorType = HypervisorType.getType(hypervisor); diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index 78af875a3be9..2703deaad649 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java @@ -1069,7 +1069,7 @@ public static boolean verifyDomainNameLabel(final String hostName, final boolean 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 && START_HOSTNAME_PATTERN.matcher(hostName).matches()) { s_logger.warn("Host name can't start with digit");