From 12c850ed2f3893313a6b31de33f47857705d9b4b Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Tue, 4 Jun 2019 03:08:31 -0300 Subject: [PATCH 1/3] KVM: Improvements on upload direct download certificates (#2995) * Improvements on upload direct download certificates * Move upload direct download certificate logic to KVM plugin * Extend unit test certificate expiration days * Add marvin tests and command to revoke certificates * Review comments * Do not include revoke certificates API --- agent/src/com/cloud/agent/Agent.java | 28 --- ...TemplateDirectDownloadCertificateCmd.java} | 6 +- ...etupDirectDownloadCertificateCommand.java} | 4 +- ...rectDownloadCertificateCommandWrapper.java | 136 +++++++++++ .../cloud/server/ManagementServerImpl.java | 4 +- .../download/DirectDownloadManagerImpl.java | 76 +++++- .../DirectDownloadManagerImplTest.java | 33 +++ .../integration/smoke/test_direct_download.py | 227 ++++++++++++++++++ 8 files changed, 472 insertions(+), 42 deletions(-) rename api/src/org/apache/cloudstack/api/command/admin/direct/download/{UploadTemplateDirectDownloadCertificate.java => UploadTemplateDirectDownloadCertificateCmd.java} (93%) rename core/src/org/apache/cloudstack/agent/directdownload/{SetupDirectDownloadCertificate.java => SetupDirectDownloadCertificateCommand.java} (89%) create mode 100644 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java create mode 100644 test/integration/smoke/test_direct_download.py diff --git a/agent/src/com/cloud/agent/Agent.java b/agent/src/com/cloud/agent/Agent.java index 500724dd5a3..e5fbdd7a2ed 100644 --- a/agent/src/com/cloud/agent/Agent.java +++ b/agent/src/com/cloud/agent/Agent.java @@ -39,7 +39,6 @@ import java.util.concurrent.atomic.AtomicInteger; import javax.naming.ConfigurationException; -import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificate; import org.apache.cloudstack.agent.lb.SetupMSListAnswer; import org.apache.cloudstack.agent.lb.SetupMSListCommand; import org.apache.cloudstack.ca.PostCertificateRenewalCommand; @@ -630,8 +629,6 @@ public class Agent implements HandlerFactory, IAgentControl { if (Host.Type.Routing.equals(_resource.getType())) { scheduleServicesRestartTask(); } - } else if (cmd instanceof SetupDirectDownloadCertificate) { - answer = setupDirectDownloadCertificate((SetupDirectDownloadCertificate) cmd); } else if (cmd instanceof SetupMSListCommand) { answer = setupManagementServerList((SetupMSListCommand) cmd); } else { @@ -683,31 +680,6 @@ public class Agent implements HandlerFactory, IAgentControl { } } - private Answer setupDirectDownloadCertificate(SetupDirectDownloadCertificate cmd) { - String certificate = cmd.getCertificate(); - String certificateName = cmd.getCertificateName(); - s_logger.info("Importing certificate " + certificateName + " into keystore"); - - final File agentFile = PropertiesUtil.findConfigFile("agent.properties"); - if (agentFile == null) { - return new Answer(cmd, false, "Failed to find agent.properties file"); - } - - final String keyStoreFile = agentFile.getParent() + "/" + KeyStoreUtils.KS_FILENAME; - - String cerFile = agentFile.getParent() + "/" + certificateName + ".cer"; - Script.runSimpleBashScript(String.format("echo '%s' > %s", certificate, cerFile)); - - String privatePasswordFormat = "sed -n '/keystore.passphrase/p' '%s' 2>/dev/null | sed 's/keystore.passphrase=//g' 2>/dev/null"; - String privatePasswordCmd = String.format(privatePasswordFormat, agentFile.getAbsolutePath()); - String privatePassword = Script.runSimpleBashScript(privatePasswordCmd); - - String importCommandFormat = "keytool -importcert -file %s -keystore %s -alias '%s' -storepass '%s' -noprompt"; - String importCmd = String.format(importCommandFormat, cerFile, keyStoreFile, certificateName, privatePassword); - Script.runSimpleBashScript(importCmd); - return new Answer(cmd, true, "Certificate " + certificateName + " imported"); - } - public Answer setupAgentKeystore(final SetupKeyStoreCommand cmd) { final String keyStorePassword = cmd.getKeystorePassword(); final long validityDays = cmd.getValidityDays(); diff --git a/api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificate.java b/api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java similarity index 93% rename from api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificate.java rename to api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java index 89c0c25c9ca..416d26452e7 100755 --- a/api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificate.java +++ b/api/src/org/apache/cloudstack/api/command/admin/direct/download/UploadTemplateDirectDownloadCertificateCmd.java @@ -35,19 +35,19 @@ import org.apache.log4j.Logger; import javax.inject.Inject; -@APICommand(name = UploadTemplateDirectDownloadCertificate.APINAME, +@APICommand(name = UploadTemplateDirectDownloadCertificateCmd.APINAME, description = "Upload a certificate for HTTPS direct template download on KVM hosts", responseObject = SuccessResponse.class, requestHasSensitiveInfo = true, responseHasSensitiveInfo = true, since = "4.11.0", authorized = {RoleType.Admin}) -public class UploadTemplateDirectDownloadCertificate extends BaseCmd { +public class UploadTemplateDirectDownloadCertificateCmd extends BaseCmd { @Inject DirectDownloadManager directDownloadManager; - private static final Logger LOG = Logger.getLogger(UploadTemplateDirectDownloadCertificate.class); + private static final Logger LOG = Logger.getLogger(UploadTemplateDirectDownloadCertificateCmd.class); public static final String APINAME = "uploadTemplateDirectDownloadCertificate"; @Parameter(name = ApiConstants.CERTIFICATE, type = BaseCmd.CommandType.STRING, required = true, length = 65535, diff --git a/core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificate.java b/core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificateCommand.java similarity index 89% rename from core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificate.java rename to core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificateCommand.java index 836b321f227..641c535f11b 100644 --- a/core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificate.java +++ b/core/src/org/apache/cloudstack/agent/directdownload/SetupDirectDownloadCertificateCommand.java @@ -20,12 +20,12 @@ package org.apache.cloudstack.agent.directdownload; import com.cloud.agent.api.Command; -public class SetupDirectDownloadCertificate extends Command { +public class SetupDirectDownloadCertificateCommand extends Command { private String certificate; private String certificateName; - public SetupDirectDownloadCertificate(String certificate, String name) { + public SetupDirectDownloadCertificateCommand(String certificate, String name) { this.certificate = certificate; this.certificateName = name; } diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java new file mode 100644 index 00000000000..97035eedd22 --- /dev/null +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupDirectDownloadCertificateCommandWrapper.java @@ -0,0 +1,136 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// +package com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.Answer; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.PropertiesUtil; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand; +import org.apache.cloudstack.utils.security.KeyStoreUtils; +import org.apache.log4j.Logger; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; + +import static org.apache.commons.lang.StringUtils.isBlank; + +@ResourceWrapper(handles = SetupDirectDownloadCertificateCommand.class) +public class LibvirtSetupDirectDownloadCertificateCommandWrapper extends CommandWrapper { + + private static final String temporaryCertFilePrefix = "CSCERTIFICATE"; + + private static final Logger s_logger = Logger.getLogger(LibvirtSetupDirectDownloadCertificateCommandWrapper.class); + + /** + * Retrieve agent.properties file + */ + private File getAgentPropertiesFile() throws FileNotFoundException { + final File agentFile = PropertiesUtil.findConfigFile("agent.properties"); + if (agentFile == null) { + throw new FileNotFoundException("Failed to find agent.properties file"); + } + return agentFile; + } + + /** + * Get the property 'keystore.passphrase' value from agent.properties file + */ + private String getKeystorePassword(File agentFile) { + String pass = null; + if (agentFile != null) { + try { + pass = PropertiesUtil.loadFromFile(agentFile).getProperty(KeyStoreUtils.KS_PASSPHRASE_PROPERTY); + } catch (IOException e) { + s_logger.error("Could not get 'keystore.passphrase' property value due to: " + e.getMessage()); + } + } + return pass; + } + + /** + * Get keystore path + */ + private String getKeyStoreFilePath(File agentFile) { + return agentFile.getParent() + "/" + KeyStoreUtils.KS_FILENAME; + } + + /** + * Import certificate from temporary file into keystore + */ + private void importCertificate(String tempCerFilePath, String keyStoreFile, String certificateName, String privatePassword) { + s_logger.debug("Importing certificate from temporary file to keystore"); + String importCommandFormat = "keytool -importcert -file %s -keystore %s -alias '%s' -storepass '%s' -noprompt"; + String importCmd = String.format(importCommandFormat, tempCerFilePath, keyStoreFile, certificateName, privatePassword); + int result = Script.runSimpleBashScriptForExitValue(importCmd); + if (result != 0) { + s_logger.debug("Certificate " + certificateName + " not imported as it already exist on keystore"); + } + } + + /** + * Create temporary file and return its path + */ + private String createTemporaryFile(File agentFile, String certificateName, String certificate) { + String tempCerFilePath = String.format("%s/%s-%s", + agentFile.getParent(), temporaryCertFilePrefix, certificateName); + s_logger.debug("Creating temporary certificate file into: " + tempCerFilePath); + int result = Script.runSimpleBashScriptForExitValue(String.format("echo '%s' > %s", certificate, tempCerFilePath)); + if (result != 0) { + throw new CloudRuntimeException("Could not create the certificate file on path: " + tempCerFilePath); + } + return tempCerFilePath; + } + + /** + * Remove temporary file + */ + private void cleanupTemporaryFile(String temporaryFile) { + s_logger.debug("Cleaning up temporary certificate file"); + Script.runSimpleBashScript("rm -f " + temporaryFile); + } + + @Override + public Answer execute(SetupDirectDownloadCertificateCommand cmd, LibvirtComputingResource serverResource) { + String certificate = cmd.getCertificate(); + String certificateName = cmd.getCertificateName(); + + try { + File agentFile = getAgentPropertiesFile(); + String privatePassword = getKeystorePassword(agentFile); + if (isBlank(privatePassword)) { + return new Answer(cmd, false, "No password found for keystore: " + KeyStoreUtils.KS_FILENAME); + } + + final String keyStoreFile = getKeyStoreFilePath(agentFile); + String temporaryFile = createTemporaryFile(agentFile, certificateName, certificate); + importCertificate(temporaryFile, keyStoreFile, certificateName, privatePassword); + cleanupTemporaryFile(temporaryFile); + } catch (FileNotFoundException | CloudRuntimeException e) { + s_logger.error("Error while setting up certificate " + certificateName, e); + return new Answer(cmd, false, e.getMessage()); + } + + return new Answer(cmd, true, "Certificate " + certificateName + " imported"); + } +} diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 38a84bb2dad..872f9fd7296 100644 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -65,7 +65,7 @@ import org.apache.cloudstack.api.command.admin.config.ListDeploymentPlannersCmd; import org.apache.cloudstack.api.command.admin.config.ListHypervisorCapabilitiesCmd; import org.apache.cloudstack.api.command.admin.config.UpdateCfgCmd; import org.apache.cloudstack.api.command.admin.config.UpdateHypervisorCapabilitiesCmd; -import org.apache.cloudstack.api.command.admin.direct.download.UploadTemplateDirectDownloadCertificate; +import org.apache.cloudstack.api.command.admin.direct.download.UploadTemplateDirectDownloadCertificateCmd; import org.apache.cloudstack.api.command.admin.domain.CreateDomainCmd; import org.apache.cloudstack.api.command.admin.domain.DeleteDomainCmd; import org.apache.cloudstack.api.command.admin.domain.ListDomainChildrenCmd; @@ -3051,7 +3051,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe cmdList.add(ReleasePodIpCmdByAdmin.class); cmdList.add(CreateManagementNetworkIpRangeCmd.class); cmdList.add(DeleteManagementNetworkIpRangeCmd.class); - cmdList.add(UploadTemplateDirectDownloadCertificate.class); + cmdList.add(UploadTemplateDirectDownloadCertificateCmd.class); // Out-of-band management APIs for admins cmdList.add(EnableOutOfBandManagementForHostCmd.class); diff --git a/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index c1ffc5ef473..d2aa67540f3 100755 --- a/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -41,6 +41,10 @@ import com.cloud.utils.exception.CloudRuntimeException; import java.net.URI; import java.net.URISyntaxException; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.CertificateExpiredException; +import java.security.cert.CertificateNotYetValidException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -50,6 +54,7 @@ import java.util.Collections; import java.util.stream.Collectors; import javax.inject.Inject; +import com.cloud.utils.security.CertificateHelper; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand.DownloadProtocol; @@ -57,7 +62,7 @@ import org.apache.cloudstack.agent.directdownload.HttpDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.MetalinkDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.NfsDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.HttpsDirectDownloadCommand; -import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificate; +import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; @@ -69,6 +74,7 @@ import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; +import sun.security.x509.X509CertImpl; public class DirectDownloadManagerImpl extends ManagerBase implements DirectDownloadManager { @@ -313,14 +319,66 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown .collect(Collectors.toList()); } + /** + * Return pretified PEM certificate + */ + protected String getPretifiedCertificate(String certificateCer) { + String cert = certificateCer.replaceAll("(.{64})", "$1\n"); + if (!cert.startsWith(BEGIN_CERT) && !cert.endsWith(END_CERT)) { + cert = BEGIN_CERT + LINE_SEPARATOR + cert + LINE_SEPARATOR + END_CERT; + } + return cert; + } + + /** + * Generate and return certificate from the string + * @throws CloudRuntimeException if the certificate is not well formed + */ + private Certificate getCertificateFromString(String certificatePem) { + try { + return CertificateHelper.buildCertificate(certificatePem); + } catch (CertificateException e) { + e.printStackTrace(); + throw new CloudRuntimeException("Cannot parse the certificate provided, please provide a PEM certificate. Error: " + e.getMessage()); + } + } + + /** + * Perform sanity of string parsed certificate + */ + protected void certificateSanity(String certificatePem) { + Certificate certificate = getCertificateFromString(certificatePem); + + if (certificate instanceof X509CertImpl) { + X509CertImpl x509Cert = (X509CertImpl) certificate; + try { + x509Cert.checkValidity(); + } catch (CertificateExpiredException | CertificateNotYetValidException e) { + String msg = "Certificate is invalid. Please provide a valid certificate. Error: " + e.getMessage(); + s_logger.error(msg); + throw new CloudRuntimeException(msg); + } + if (x509Cert.getSubjectDN() != null) { + s_logger.debug("Valid certificate for domain name: " + x509Cert.getSubjectDN().getName()); + } + } + } + @Override - public boolean uploadCertificateToHosts(String certificateCer, String certificateName, String hypervisor) { + public boolean uploadCertificateToHosts(String certificateCer, String alias, String hypervisor) { HypervisorType hypervisorType = HypervisorType.getType(hypervisor); List hosts = getRunningHostsToUploadCertificate(hypervisorType); + + String certificatePem = getPretifiedCertificate(certificateCer); + certificateSanity(certificatePem); + + s_logger.info("Attempting to upload certificate: " + alias + " to " + hosts.size() + " hosts"); if (CollectionUtils.isNotEmpty(hosts)) { for (HostVO host : hosts) { - if (!uploadCertificate(certificateCer, certificateName, host.getId())) { - throw new CloudRuntimeException("Uploading certificate " + certificateName + " failed on host: " + host.getId()); + if (!uploadCertificate(certificatePem, alias, host.getId())) { + String msg = "Could not upload certificate " + alias + " on host: " + host.getName() + " (" + host.getUuid() + ")"; + s_logger.error(msg); + throw new CloudRuntimeException(msg); } } } @@ -331,14 +389,18 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown * Upload and import certificate to hostId on keystore */ protected boolean uploadCertificate(String certificate, String certificateName, long hostId) { - String cert = certificate.replaceAll("(.{64})", "$1\n"); - final String prettified_cert = BEGIN_CERT + LINE_SEPARATOR + cert + LINE_SEPARATOR + END_CERT; - SetupDirectDownloadCertificate cmd = new SetupDirectDownloadCertificate(prettified_cert, certificateName); + SetupDirectDownloadCertificateCommand cmd = new SetupDirectDownloadCertificateCommand(certificate, certificateName); Answer answer = agentManager.easySend(hostId, cmd); if (answer == null || !answer.getResult()) { + String msg = "Certificate " + certificateName + " could not be added to host " + hostId; + if (answer != null) { + msg += " due to: " + answer.getDetails(); + } + s_logger.error(msg); return false; } s_logger.info("Certificate " + certificateName + " successfully uploaded to host: " + hostId); return true; } + } diff --git a/server/test/org/apache/cloudstack/direct/download/DirectDownloadManagerImplTest.java b/server/test/org/apache/cloudstack/direct/download/DirectDownloadManagerImplTest.java index 59405998f9f..5082500a9ae 100644 --- a/server/test/org/apache/cloudstack/direct/download/DirectDownloadManagerImplTest.java +++ b/server/test/org/apache/cloudstack/direct/download/DirectDownloadManagerImplTest.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.direct.download; import com.cloud.agent.AgentManager; import com.cloud.host.dao.HostDao; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand.DownloadProtocol; import org.junit.Assert; import org.junit.Before; @@ -50,6 +51,26 @@ public class DirectDownloadManagerImplTest { private static final String HTTP_HEADER_2 = "Accept-Encoding"; private static final String HTTP_VALUE_2 = "gzip"; + private static final String VALID_CERTIFICATE = + "MIIDSzCCAjMCFDa0LoW+1O8/cEwCI0nIqfl8c1TLMA0GCSqGSIb3DQEBCwUAMGEx\n" + + "CzAJBgNVBAYTAkNTMQswCQYDVQQIDAJDUzELMAkGA1UEBwwCQ1MxCzAJBgNVBAoM\n" + + "AkNTMQswCQYDVQQLDAJDUzELMAkGA1UEAwwCQ1MxETAPBgkqhkiG9w0BCQEWAkNT\n" + + "MCAXDTE5MDQyNDE1NTIzNVoYDzIwOTgwOTE1MTU1MjM1WjBhMQswCQYDVQQGEwJD\n" + + "UzELMAkGA1UECAwCQ1MxCzAJBgNVBAcMAkNTMQswCQYDVQQKDAJDUzELMAkGA1UE\n" + + "CwwCQ1MxCzAJBgNVBAMMAkNTMREwDwYJKoZIhvcNAQkBFgJDUzCCASIwDQYJKoZI\n" + + "hvcNAQEBBQADggEPADCCAQoCggEBAKstLRcMGCo6+2hojRMjEuuimnWp27yfYhDU\n" + + "w/Cj03MJe/KCOhwsDqX82QNIr/bNtLdFf2ZJEUQd08sLLlHeUy9y5aOcxt9SGx2j\n" + + "xolqO4MBL7BW3dklO0IvjaEfBeFP6udz8ajeVur/iPPZb2Edd0zlXuHvDozfQisv\n" + + "bpuJImnTUVx0ReCXP075PBGvlqQXW2uEht+E/w3H8/2rra3JFV6J5xc77KyQSq2t\n" + + "1+2ZU7PJiy/rppXf5rjTvNm6ydfag8/av7lcgs2ntdkK4koAmkmROhAwNonlL7cD\n" + + "xIC83cKOqOFiQXSwr1IgoLf7zBNafKoTlSb/ev6Zt18BXEMLGpkCAwEAATANBgkq\n" + + "hkiG9w0BAQsFAAOCAQEAVS5uWZRz2m3yx7EUQm47RTMW5WMXU4pI8D+N5WZ9xubY\n" + + "OqtU3r2OAYpfL/QO8iT7jcqNYGoDqe8ZjEaNvfxiTG8cOI6TSXhKBG6hjSaSFQSH\n" + + "OZ5mfstM36y/3ENFh6JCJ2ao1rgWSbfDRyAaHuvt6aCkaV6zRq2OMEgoJqZSgwxL\n" + + "QO230xa2hYgKXOePMVZyHFA2oKJtSOc3jCke9Y8zDUwm0McGdMRBD8tVB0rcaOqQ\n" + + "0PlDLjB9sQuhhLu8vjdgbznmPbUmMG7JN0yhT1eJbIX5ImXyh0DoTwiaGcYwW6Sq\n" + + "YodjXACsC37xaQXAPYBiaAs4iI80TJSx1DVFO1LV0g=="; + @Before public void setUp() { } @@ -103,4 +124,16 @@ public class DirectDownloadManagerImplTest { Map headers = manager.getHeadersFromDetails(details); Assert.assertTrue(headers.isEmpty()); } + + @Test + public void testCertificateSanityValidCertificate() { + String pretifiedCertificate = manager.getPretifiedCertificate(VALID_CERTIFICATE); + manager.certificateSanity(pretifiedCertificate); + } + + @Test(expected = CloudRuntimeException.class) + public void testCertificateSanityInvalidCertificate() { + String pretifiedCertificate = manager.getPretifiedCertificate(VALID_CERTIFICATE + "xxx"); + manager.certificateSanity(pretifiedCertificate); + } } diff --git a/test/integration/smoke/test_direct_download.py b/test/integration/smoke/test_direct_download.py new file mode 100644 index 00000000000..65117f97f8c --- /dev/null +++ b/test/integration/smoke/test_direct_download.py @@ -0,0 +1,227 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" Test for Direct Downloads of Templates and ISOs +""" +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import (cleanup_resources) +from marvin.lib.base import (ServiceOffering, + NetworkOffering, + Network, + Template, + VirtualMachine) +from marvin.lib.common import (get_pod, + get_zone) +from nose.plugins.attrib import attr +from marvin.cloudstackAPI import uploadTemplateDirectDownloadCertificate +from marvin.lib.decoratorGenerators import skipTestIf + + +class TestUploadDirectDownloadCertificates(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestUploadDirectDownloadCertificates, cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.hypervisor = cls.testClient.getHypervisorInfo() + cls.dbclient = cls.testClient.getDbConnection() + cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.pod = get_pod(cls.apiclient, cls.zone.id) + cls.services = cls.testClient.getParsedTestDataConfig() + + cls._cleanup = [] + cls.hypervisorNotSupported = False + if cls.hypervisor.lower() not in ['kvm', 'lxc']: + cls.hypervisorNotSupported = True + + if not cls.hypervisorNotSupported: + cls.certificates = { + "expired": "MIIDSTCCAjECFDi8s70TWFhwVN9cj67RJoAF99c8MA0GCSqGSIb3DQEBCwUAMGExCzAJBgNVBAYTAkNTMQswCQYDVQQIDAJDUzELMAkGA1UEBwwCQ1MxCzAJBgNVBAoMAkNTMQswCQYDVQQLDAJDUzELMAkGA1UEAwwCQ1MxETAPBgkqhkiG9w0BCQEWAkNTMB4XDTE5MDQyNDE1NTQxM1oXDTE5MDQyMjE1NTQxM1owYTELMAkGA1UEBhMCQ1MxCzAJBgNVBAgMAkNTMQswCQYDVQQHDAJDUzELMAkGA1UECgwCQ1MxCzAJBgNVBAsMAkNTMQswCQYDVQQDDAJDUzERMA8GCSqGSIb3DQEJARYCQ1MwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCrLS0XDBgqOvtoaI0TIxLropp1qdu8n2IQ1MPwo9NzCXvygjocLA6l/NkDSK/2zbS3RX9mSRFEHdPLCy5R3lMvcuWjnMbfUhsdo8aJajuDAS+wVt3ZJTtCL42hHwXhT+rnc/Go3lbq/4jz2W9hHXdM5V7h7w6M30IrL26biSJp01FcdEXglz9O+TwRr5akF1trhIbfhP8Nx/P9q62tyRVeiecXO+yskEqtrdftmVOzyYsv66aV3+a407zZusnX2oPP2r+5XILNp7XZCuJKAJpJkToQMDaJ5S+3A8SAvN3CjqjhYkF0sK9SIKC3+8wTWnyqE5Um/3r+mbdfAVxDCxqZAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAG/R9sJ2pFbu35MliIJIhWkwP7FeP/7gYCNvOXFt6vVGXmcOwuw9WGBxsmsGESQRB4+NnJFjyGQ1Ck+ps5XRRMizyvq6bCQxVuC5M+vYS4J0q8YoL0RJ20pN9iwTsosZjSEKmfUlVgsufqCG2nyusV71LSaQU6f/bylJcJkKwGUhThExh+PVLZ66H5cF4/SzuK6WzWnj5p6+YX8TP+qPUkXN1mapgVKfVMo6mqLsH+eLKH+zqdy5ZZ5znNSbJFgHufYbEFlutTaxHEvKNMEgMCFkFGiyPwRuD6oaPnZFquJLh/mBZOLogpxVD5v20AcUTANtbXSlPaqOnEQFcbiVCb8=", + "invalid": "XXXXXXXXXXXXXXXXXXXXXXXXXXXX", + "valid": "MIIDSzCCAjMCFDa0LoW+1O8/cEwCI0nIqfl8c1TLMA0GCSqGSIb3DQEBCwUAMGExCzAJBgNVBAYTAkNTMQswCQYDVQQIDAJDUzELMAkGA1UEBwwCQ1MxCzAJBgNVBAoMAkNTMQswCQYDVQQLDAJDUzELMAkGA1UEAwwCQ1MxETAPBgkqhkiG9w0BCQEWAkNTMCAXDTE5MDQyNDE1NTIzNVoYDzIwOTgwOTE1MTU1MjM1WjBhMQswCQYDVQQGEwJDUzELMAkGA1UECAwCQ1MxCzAJBgNVBAcMAkNTMQswCQYDVQQKDAJDUzELMAkGA1UECwwCQ1MxCzAJBgNVBAMMAkNTMREwDwYJKoZIhvcNAQkBFgJDUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKstLRcMGCo6+2hojRMjEuuimnWp27yfYhDUw/Cj03MJe/KCOhwsDqX82QNIr/bNtLdFf2ZJEUQd08sLLlHeUy9y5aOcxt9SGx2jxolqO4MBL7BW3dklO0IvjaEfBeFP6udz8ajeVur/iPPZb2Edd0zlXuHvDozfQisvbpuJImnTUVx0ReCXP075PBGvlqQXW2uEht+E/w3H8/2rra3JFV6J5xc77KyQSq2t1+2ZU7PJiy/rppXf5rjTvNm6ydfag8/av7lcgs2ntdkK4koAmkmROhAwNonlL7cDxIC83cKOqOFiQXSwr1IgoLf7zBNafKoTlSb/ev6Zt18BXEMLGpkCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAVS5uWZRz2m3yx7EUQm47RTMW5WMXU4pI8D+N5WZ9xubYOqtU3r2OAYpfL/QO8iT7jcqNYGoDqe8ZjEaNvfxiTG8cOI6TSXhKBG6hjSaSFQSHOZ5mfstM36y/3ENFh6JCJ2ao1rgWSbfDRyAaHuvt6aCkaV6zRq2OMEgoJqZSgwxLQO230xa2hYgKXOePMVZyHFA2oKJtSOc3jCke9Y8zDUwm0McGdMRBD8tVB0rcaOqQ0PlDLjB9sQuhhLu8vjdgbznmPbUmMG7JN0yhT1eJbIX5ImXyh0DoTwiaGcYwW6SqYodjXACsC37xaQXAPYBiaAs4iI80TJSx1DVFO1LV0g==" + } + + return + + @classmethod + def tearDownClass(cls): + try: + cleanup_resources(cls.apiclient, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + return + + def tearDown(self): + try: + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @skipTestIf("hypervisorNotSupported") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_01_sanity_check_on_certificates(self): + """Test Verify certificates before uploading to KVM hosts + """ + + # Validate the following + # 1. Invalid certificates cannot be uploaded to hosts for direct downloads + # 2. Expired certificates cannot be uploaded to hosts for direct downloads + + cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() + cmd.hypervisor = self.hypervisor + cmd.name = "marvin-test-verify-certs" + cmd.certificate = self.certificates["invalid"] + + invalid_cert_uploadFails = False + expired_cert_upload_fails = False + try: + self.apiclient.uploadTemplateDirectDownloadCertificate(cmd) + self.fail("Invalid certificate must not be uploaded") + except Exception as e: + invalid_cert_uploadFails = True + + cmd.certificate = self.certificates["expired"] + try: + self.apiclient.uploadTemplateDirectDownloadCertificate(cmd) + self.fail("Expired certificate must not be uploaded") + except Exception as e: + expired_cert_upload_fails = True + + self.assertTrue(invalid_cert_uploadFails and expired_cert_upload_fails, + "Invalid or expired certificates must not be uploaded") + return + + @skipTestIf("hypervisorNotSupported") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_02_upload_direct_download_certificates(self): + """Test Upload certificates to KVM hosts for direct download + """ + + # Validate the following + # 1. Valid certificates are uploaded to hosts + + cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() + cmd.hypervisor = self.hypervisor + cmd.name = "marvin-test-verify-certs" + cmd.certificate = self.certificates["valid"] + + try: + self.apiclient.uploadTemplateDirectDownloadCertificate(cmd) + except Exception as e: + self.fail("Valid certificate must be uploaded") + + return + + +class TestDirectDownloadTemplates(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestDirectDownloadTemplates, cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.hypervisor = cls.testClient.getHypervisorInfo() + cls.dbclient = cls.testClient.getDbConnection() + cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.pod = get_pod(cls.apiclient, cls.zone.id) + cls.services = cls.testClient.getParsedTestDataConfig() + + cls._cleanup = [] + cls.hypervisorNotSupported = False + if cls.hypervisor.lower() not in ['kvm', 'lxc']: + cls.hypervisorNotSupported = True + + if not cls.hypervisorNotSupported: + cls.services["test_templates"]["kvm"]["directdownload"] = "true" + cls.template = Template.register(cls.apiclient, cls.services["test_templates"]["kvm"], + zoneid=cls.zone.id, hypervisor=cls.hypervisor) + cls._cleanup.append(cls.template) + + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + cls.services["virtual_machine"]["template"] = cls.template.id + cls.services["virtual_machine"]["hypervisor"] = cls.hypervisor + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.services["service_offerings"]["tiny"] + ) + cls._cleanup.append(cls.service_offering) + cls.network_offering = NetworkOffering.create( + cls.apiclient, + cls.services["l2-network_offering"], + ) + cls.network_offering.update(cls.apiclient, state='Enabled') + cls.services["network"]["networkoffering"] = cls.network_offering.id + cls.l2_network = Network.create( + cls.apiclient, + cls.services["l2-network"], + zoneid=cls.zone.id, + networkofferingid=cls.network_offering.id + ) + cls._cleanup.append(cls.l2_network) + cls._cleanup.append(cls.network_offering) + return + + @classmethod + def tearDownClass(cls): + try: + cleanup_resources(cls.apiclient, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + return + + def tearDown(self): + try: + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @skipTestIf("hypervisorNotSupported") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_01_deploy_vm_from_direct_download_template(self): + """Test Deploy VM from direct download template + """ + + # Validate the following + # 1. Register direct download template + # 2. Deploy VM from direct download template + + vm = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + serviceofferingid=self.service_offering.id, + networkids=self.l2_network.id + ) + self.assertEqual( + vm.state, + "Running", + "Check VM deployed from direct download template is running" + ) + self.cleanup.append(vm) + return From 42501ceecf90683440c97df740c5fa62f09df4f6 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 4 Jun 2019 15:53:51 +0530 Subject: [PATCH 2/3] ssvm: apply MTU value on storage/management nic if available (#3370) If mtu= value is defined in the parameters received by the SSVM agent per the secstorage.vm.mtu.size setting, it applies the MTU setting on eth1 which is the storage/management nic. Fixes #3369 Signed-off-by: Rohit Yadav --- .../resource/NfsSecondaryStorageResource.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index 46fa1707a03..be68f63ed91 100644 --- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -257,6 +257,30 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S _inSystemVM = inSystemVM; } + /** + * Applies MTU per the "mtu" value from params + * @param params + */ + public static void applyMtu(Map params) { + if (params == null || params.get("mtu") == null) { + return; + } + final Integer mtu = NumbersUtil.parseInt((String) params.get("mtu"), 0); + if (mtu > 0) { + final Script command = new Script("/sbin/ip", s_logger); + command.add("link"); + command.add("set"); + command.add("dev"); + command.add("eth1"); + command.add("mtu"); + command.add(String.valueOf(mtu)); + final String result = command.execute(); + if (result != null) { + s_logger.warn("Error in setting MTU on storage/management nic: " + result); + } + } + } + /** * Retrieve converted "nfsVersion" value from params * @param params @@ -2605,6 +2629,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S } startAdditionalServices(); + applyMtu(params); _params.put("install.numthreads", "50"); _params.put("secondary.storage.vm", "true"); _nfsVersion = retrieveNfsVersionFromParams(params); From c94ee1454d3ecdf1bf371fd18c8e3ec0631d3092 Mon Sep 17 00:00:00 2001 From: Vladimir Melnik Date: Tue, 4 Jun 2019 13:34:45 +0300 Subject: [PATCH 3/3] kvm: suspend a VM before snapshot deletion (see PR #3193) (#3194) To make sure that a qemu2-image won't be corrupted by the snapshot deletion procedure which is being performed after copying the snapshot to a secondary store, I'd propose to put a VM in to suspended state. Additional reference: https://bugzilla.redhat.com/show_bug.cgi?id=920020#c5 Fixes #3193 --- .../cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 6 ++++++ ui/l10n/en.js | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 36be2d39a2e..53a2278c163 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -996,6 +996,12 @@ public class KVMStorageProcessor implements StorageProcessor { primaryStore.getUuid()); if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryStorage.isExternalSnapshot()) { final DomainSnapshot snap = vm.snapshotLookupByName(snapshotName); + try { + vm.suspend(); + } catch(final Exception e) { + s_logger.debug("Failed to suspend the VM: " + e); + throw e; + } snap.delete(0); /* diff --git a/ui/l10n/en.js b/ui/l10n/en.js index 86a860616e0..b9f36822b7a 100644 --- a/ui/l10n/en.js +++ b/ui/l10n/en.js @@ -1922,7 +1922,7 @@ var dictionary = {"ICMP.code":"ICMP Code", "message.action.take.snapshot":"Please confirm that you want to take a snapshot of this volume.", "message.action.unmanage.cluster":"Please confirm that you want to unmanage the cluster.", "message.action.vmsnapshot.create":"Please confirm that you want to take a snapshot of this instance.
Please notice that the instance will be paused during the snapshoting, and resumed after snapshotting, if it runs on KVM.", -"message.action.vmsnapshot.delete":"Please confirm that you want to delete this VM snapshot.", +"message.action.vmsnapshot.delete":"Please confirm that you want to delete this VM snapshot.
Please notice that the instance will be paused before the snapshot deletion, and resumed after deletion, if it runs on KVM.", "message.action.vmsnapshot.revert":"Revert VM snapshot", "message.activate.project":"Are you sure you want to activate this project?", "message.add.VPN.gateway":"Please confirm that you want to add a VPN Gateway",